New issue
Advanced search Search tips

Issue 899525 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

[Regression r522877] DevTools: can't resize devtools on Mac when docked to right

Project Member Reported by pfeldman@google.com, Oct 28

Issue description

Pretty much impossible to work with the tool because of that. Could you please fix/reassign?
 
This was regressed by https://chromium-review.googlesource.com/c/chromium/src/+/814696. Probably similar to  issue 851876 . Locally reverting fixes the issue.
Cc: dgozman@chromium.org
Labels: -Pri-3 Pri-1
Owner: dtapu...@chromium.org
Dave, this is a pretty big issue for DevTools users on Mac. Perhaps, we should find another way to fix the framesets issue, which does not regress DevTools? Meanwhile, I think reverting the patch will be beneficial to many users. WDYT?
Cc: dtapu...@chromium.org
Owner: nzolghadr@chromium.org
Since this has been in the code base for a year I don't think reverting it is the best course right now as it causes other regreasions. I'd let Navid debug it for some time before reverting. 

Assigning to Navid since I don't work on input anymore. 
Labels: ReleaseBlock-Stable
Summary: [Regression r522877] DevTools: can't resize devtools on Mac when docked to right (was: DevTools: can't resize devtools on Mac when docked to right)
Adding my 2 cents: I have not been doing web development for quite some time, but I tried recently and this bug is a major inconvenience to say the least. Asking you to prioritize it.
Pavel, can you elaborate on steps to reproduce the problem. I just tested with Canary 73.0.3629.0 on Mac and I set the devtools to stick to the right of the screen. I was able to resize it. Although when it was resizing sometimes the icons in the devtools would disappear. Not sure what that is. But nevertheless I was able to resize it.
>> "the icons in the devtools would disappear"

That's what this bug is about. When you are moving the cursor left to right (from being over the page towards the devtools), the resize cursor never appears. Revering r522877 makes it appear.
I didn't mean the mouse cursor. I meant the UI icons in the devtools. Like the Elements/Console/... tab content.
But the mouse cursor does change to resize at the border of devtools when I move it from the page. Is that reproducible for you on this latest Chrome Canary?
Yeah, it is reproducible. Note, as it is saying in the title, this is Mac-specific.

Icons are probably blinking because another compositor bug :)
I was trying on Mac as well. But now I see. The resize cursor doesn't show when coming from page to the border (left to right as you mentioned when devtools docked to the right) but it does show but when it comes from the devtools surface to the border (i.e. right to left). Is that correct?
Project Member

Comment 10 by sheriffbot@chromium.org, Dec 4

Cc: pfeldman@chromium.org
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Did you have a chance to look at it? I'd like to merge the fix during the first week of Beta if possible.
Status: Started (was: Assigned)
I have started working on the issue and found a potential fix. Are you targeting 72?
Labels: M-72
nzolghadr@,
Please land a fix as per C#12 & C#13.
Thanks..!

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/293bb860440af3627941ee6017f812d2a296e2e9

commit 293bb860440af3627941ee6017f812d2a296e2e9
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Wed Dec 12 18:15:41 2018

Reset last known cursor on mouse leave

Reset last known cursor in RenderWidgetInputHandler
upon receiving a mouse leave event so the very next
set cursor command in this widget always sets the
cursor.

This change originally failed
CursorUpdateReceivedFromCrossSiteIframe
test and fixes were added to EventHandler to stop
sending set cursor message when mouse is on a
remote embedded iframe by the embedder.

Bug:  899525 
Change-Id: I0a7114bccc7c3087b4993a212e5d5a8c1b39a03b
Reviewed-on: https://chromium-review.googlesource.com/c/1365085
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615964}
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/browser/site_per_process_hit_test_browsertest.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/renderer/input/render_widget_input_handler.h
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/renderer/render_widget.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/renderer/render_widget.h
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/third_party/blink/renderer/core/input/event_handler.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/third_party/blink/renderer/core/input/event_handling_util.cc
[modify] https://crrev.com/293bb860440af3627941ee6017f812d2a296e2e9/third_party/blink/renderer/core/input/event_handling_util.h

Pavel, I have tested on latest Canary and the issue seems to be resolved. Do you mind also checking this before I merge this into the branch?
Labels: Merge-Request-72
Project Member

Comment 18 by sheriffbot@chromium.org, Dec 13

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pavel - please confirm once you've tested this. 
pavel any update on this test results?

MacOS Canary is working as expected, thank you for fixing this!
Labels: -Merge-Review-72 Merge-Approved-72
approving for M72 branch :3626
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 22a7e5ea73f990c754305d1b413fcd93ccddb971 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: -Merge-Approved-72 Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/22a7e5ea73f990c754305d1b413fcd93ccddb971

Commit: 22a7e5ea73f990c754305d1b413fcd93ccddb971
Author: nzolghadr@chromium.org
Commiter: nzolghadr@chromium.org
Date: 2018-12-19 19:39:30 +0000 UTC

Reset last known cursor on mouse leave

Reset last known cursor in RenderWidgetInputHandler
upon receiving a mouse leave event so the very next
set cursor command in this widget always sets the
cursor.

This change originally failed
CursorUpdateReceivedFromCrossSiteIframe
test and fixes were added to EventHandler to stop
sending set cursor message when mouse is on a
remote embedded iframe by the embedder.

Bug:  899525 
Change-Id: I0a7114bccc7c3087b4993a212e5d5a8c1b39a03b
Reviewed-on: https://chromium-review.googlesource.com/c/1365085
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615964}(cherry picked from commit 293bb860440af3627941ee6017f812d2a296e2e9)
Reviewed-on: https://chromium-review.googlesource.com/c/1384623
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#466}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 19

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22a7e5ea73f990c754305d1b413fcd93ccddb971

commit 22a7e5ea73f990c754305d1b413fcd93ccddb971
Author: Navid Zolghadr <nzolghadr@chromium.org>
Date: Wed Dec 19 19:39:30 2018

Reset last known cursor on mouse leave

Reset last known cursor in RenderWidgetInputHandler
upon receiving a mouse leave event so the very next
set cursor command in this widget always sets the
cursor.

This change originally failed
CursorUpdateReceivedFromCrossSiteIframe
test and fixes were added to EventHandler to stop
sending set cursor message when mouse is on a
remote embedded iframe by the embedder.

Bug:  899525 
Change-Id: I0a7114bccc7c3087b4993a212e5d5a8c1b39a03b
Reviewed-on: https://chromium-review.googlesource.com/c/1365085
Commit-Queue: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615964}(cherry picked from commit 293bb860440af3627941ee6017f812d2a296e2e9)
Reviewed-on: https://chromium-review.googlesource.com/c/1384623
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#466}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/browser/site_per_process_hit_test_browsertest.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/renderer/input/render_widget_input_handler.h
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/renderer/render_widget.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/renderer/render_widget.h
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/third_party/blink/renderer/core/input/event_handler.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/third_party/blink/renderer/core/input/event_handling_util.cc
[modify] https://crrev.com/22a7e5ea73f990c754305d1b413fcd93ccddb971/third_party/blink/renderer/core/input/event_handling_util.h

Labels: -CommitLog-Audit-Violation -Merge-Without-Approval
Status: Fixed (was: Started)
Cc: nyerramilli@chromium.org rbasuvula@chromium.org nzolghadr@chromium.org
 Issue 851876  has been merged into this issue.

Sign in to add a comment