Issue metadata
Sign in to add a comment
|
Regression:Unable to close the overlay while clicking on the middle of the close(x) button |
||||||||||||||||||||||||
Issue descriptionChrome Version: 59.0.3065.0 OS: Ubuntu 14.04 What steps will reproduce the problem? (1)Launch chrome and navigate to the chrome web store (2)add any app and overlay opens and place the mouse pointer in the centre of X(close)button click and observe Expected Result:While clicking on the centre of the close button(x) should close the overlay Actual Result:instead clicking on the centre of the close button(x) is not closing the overlay will provide bisect info soon
,
Apr 7 2017
,
Apr 7 2017
This is regression issue broken in M-58 Manual bisect info: ================= Good Build:58.0.2998.0 Bad Build:58.0.2999.0 Issue is not able to repro in windows OS
,
Apr 7 2017
This doesn't happen consistently and is edge case hence moving this to RB-Stable for M-58 based on the above regression range.
,
Apr 7 2017
Using the per-revision bisect providing the bisect results, Good build:58.0.2998.0 (Revision:447146). Bad build:58.0.2999.0 (Revision:447413). You are probably looking for a change made after 447336 (known good), but no later than 447337 (first known bad). CHANGE-LOG URL: --------------- https://chromium.googlesource.com/chromium/src/+log/0da7a38852b601f5964d44a374f86ea0b30b6ef0..5c8c2e7b510f494dbde1a63f4fda119701a727db From the CL above, assigning the issue to the concern owner @pkasting: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. Review-Url: https://codereview.chromium.org/2667903002 Note :Linux specific issue and Able to reproduce in latest Canary #59.0.3065.0
,
Apr 7 2017
Issue 709393 has been merged into this issue.
,
Apr 10 2017
Evan, you know more about controls, and Linux-specific issues, than I do (and I don't have a Linux box) -- can you understand how changing from a LabelButton to an ImageButton for the bubble close button could have the effect in the video in comment 3?
,
Apr 10 2017
,
Apr 10 2017
the good news is that I can reproduce it on Linux. I don't know offhand why it's happening. I kind of suspect the regression range may be wrong and this is a result of the changes to BubbleFrameView layout. Will look into it more tomorrow.
,
Apr 11 2017
I see this bug on cros as well. The close button (part of the frame view) overlaps the client view and the client view gets the first crack at mouse events.[1] I think the difference here is that the button is getting sized smaller because a LabelButton reserves enough height for the label based on its font list (even though the label is empty), but an ImageButton has no such label. Note that the extensions dialog is a weird one because it doesn't use the BubbleFrameView's title, and we have to make sure that layout is correct when there's only a close button. [1] https://cs.chromium.org/chromium/src/ui/views/window/non_client_view.cc?rcl=cd699c91c02f773fb450281afacf148e1954c146&l=29
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ced9f3c7e9534b6b730d3b265e39a0fe5b17dae commit 4ced9f3c7e9534b6b730d3b265e39a0fe5b17dae Author: estade <estade@chromium.org> Date: Fri Apr 14 16:01:29 2017 Fix layout of BubbleFrameView when there's only a close button in the title row. Push the client area down slightly so it doesn't overlap close. This is a bandaid until bug 702196 is fixed. BUG= 709380 ,702196 Review-Url: https://codereview.chromium.org/2807243004 Cr-Commit-Position: refs/heads/master@{#464731} [modify] https://crrev.com/4ced9f3c7e9534b6b730d3b265e39a0fe5b17dae/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/4ced9f3c7e9534b6b730d3b265e39a0fe5b17dae/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/4ced9f3c7e9534b6b730d3b265e39a0fe5b17dae/ui/views/window/dialog_delegate_unittest.cc
,
Apr 14 2017
this happens consistently for this dialog across at least Linux and Chrome OS, and probably Windows. It occurs on m58 and m59. It may need to be merged to m58 (if deemed appropriate) and m59 (if it missed the branch --- I don't see a branch email at the moment). Assigning to pkasting to deal with merging.
,
Apr 14 2017
Looks like the fix here got reverted. Once we do land this, yes, we should merge to M58.
,
Apr 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77227cc6631117209eb6cfbbc5c78bf0d3c8a4b4 commit 77227cc6631117209eb6cfbbc5c78bf0d3c8a4b4 Author: alexmos <alexmos@chromium.org> Date: Fri Apr 14 19:25:03 2017 Revert of Fix layout of BubbleFrameView when there's only a close button in the (patchset #5 id:40002 of https://codereview.chromium.org/2807243004/ ) Reason for revert: Breaks DialogTest.AcceptAndCancel on a couple of bots (https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/59105 and https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/24880). Original issue's description: > Fix layout of BubbleFrameView when there's only a close button in the > title row. > > Push the client area down slightly so it doesn't overlap close. This > is a bandaid until bug 702196 is fixed. > > BUG= 709380 ,702196 TBR=tapted@chromium.org,pkasting@chromium.org,bsep@chromium.org,estade@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 709380 ,702196 Review-Url: https://codereview.chromium.org/2821463004 Cr-Commit-Position: refs/heads/master@{#464773} [modify] https://crrev.com/77227cc6631117209eb6cfbbc5c78bf0d3c8a4b4/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/77227cc6631117209eb6cfbbc5c78bf0d3c8a4b4/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/77227cc6631117209eb6cfbbc5c78bf0d3c8a4b4/ui/views/window/dialog_delegate_unittest.cc
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eed63750baeef51b69420b637d4e42c6be2c446d commit eed63750baeef51b69420b637d4e42c6be2c446d Author: estade <estade@chromium.org> Date: Mon Apr 17 21:39:35 2017 Re-land 4ced9f3c7e9534b6b730d: Fix layout of BubbleFrameView when there's only a close button in the title row. Push the client area down slightly so it doesn't overlap close. This is a bandaid until bug 702196 is fixed. BUG= 709380 , 702196 TBR=tapted@chromium.org Review-Url: https://codereview.chromium.org/2821893003 Cr-Commit-Position: refs/heads/master@{#465029} [modify] https://crrev.com/eed63750baeef51b69420b637d4e42c6be2c446d/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/eed63750baeef51b69420b637d4e42c6be2c446d/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/eed63750baeef51b69420b637d4e42c6be2c446d/ui/views/window/dialog_delegate_unittest.cc
,
Apr 17 2017
,
Apr 18 2017
This bug requires manual review: Only 6 days from stable, we might already have a stable candidate build Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 18 2017
+abdulsyed@ for M58 merge review
,
Apr 18 2017
There is a huge risk in taking a change post branch as we're cutting M58 stable RC today. Can you please confirm if this is critical, and if yes, has this been baked into canary, tested, and is there enough unit test coverage for this?
,
Apr 18 2017
I'm going to bow out of the merge discussion and let pkasting@ voice his thoughts.
,
Apr 18 2017
I think this is too risky to merge to M58 at this point. It's a frustrating regression and we're going to get bugs filed on it, but we haven't baked it yet, and we need to. Maybe we can merge into the next M58 point release, if one is planned?
,
Apr 18 2017
Verified in canary - 60.0.3074.0 (Official Build) (64-bit), Ubuntu Trusty. Working as intended. Looping to Geetha and Song to verify in CrOS.
,
Apr 18 2017
Thanks for confirming estade@ and pkasting@ - If there are any respins for M58 (ideally there shouldn't be), we can take this change in. For now, I'll leave this open with Merge-Review-58 label on.
,
Apr 18 2017
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 18 2017
Merging to M59 should be fine
,
Apr 18 2017
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 18 2017
As per #24, ideally we may not have any M58 respin. Given this is a regression in M58, pkasting@ please confirm whether its ok to go for full stable release with this bug.
,
Apr 18 2017
I think it's probably OK. Either route makes me really uncomfortable.
,
Apr 19 2017
Approving this for M59.
,
Apr 19 2017
Please merge to M59 branch 3071 before 5:00 PM PT Today (04/19) so we can pick it up for tomorrow's M59 dev release. Thank you.
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f3e136ba02bfc3735028a34ae39ee4507f9ffc8 commit 3f3e136ba02bfc3735028a34ae39ee4507f9ffc8 Author: Peter Kasting <pkasting@chromium.org> Date: Wed Apr 19 23:30:14 2017 Re-land 4ced9f3c7e9534b6b730d: Fix layout of BubbleFrameView when there's only a close button in the title row. Push the client area down slightly so it doesn't overlap close. This is a bandaid until bug 702196 is fixed. BUG= 709380 , 702196 TBR=tapted@chromium.org Review-Url: https://codereview.chromium.org/2821893003 Cr-Commit-Position: refs/heads/master@{#465029} (cherry picked from commit eed63750baeef51b69420b637d4e42c6be2c446d) Review-Url: https://codereview.chromium.org/2826323002 . Cr-Commit-Position: refs/branch-heads/3071@{#71} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/3f3e136ba02bfc3735028a34ae39ee4507f9ffc8/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/3f3e136ba02bfc3735028a34ae39ee4507f9ffc8/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/3f3e136ba02bfc3735028a34ae39ee4507f9ffc8/ui/views/window/dialog_delegate_unittest.cc
,
Apr 20 2017
Verified this issue on Ubuntu 14.04 using chrome latest Dev #59.0.3071.15 by following steps mentioned in the original comment, Observed while clicking on the center of the close button(x) the dialog box closes as expected. Hence adding TE-Verified label.
,
Apr 21 2017
Verified the issue on Chrome 59.0.3071.15/9460.5.0 - Peppy, Paine.
,
Apr 24 2017
Given we are still spinning a 58 for Chrome OS, and this is verified on 59, it sounds like this is something we should merge. Merge approved for 58.
,
Apr 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77b32554d91e8fd5a1f093038926d262a4bda5a0 commit 77b32554d91e8fd5a1f093038926d262a4bda5a0 Author: Peter Kasting <pkasting@chromium.org> Date: Mon Apr 24 17:34:39 2017 Re-land 4ced9f3c7e9534b6b730d: Fix layout of BubbleFrameView when there's only a close button in the title row. Push the client area down slightly so it doesn't overlap close. This is a bandaid until bug 702196 is fixed. BUG= 709380 , 702196 TBR=tapted@chromium.org Review-Url: https://codereview.chromium.org/2821893003 Cr-Commit-Position: refs/heads/master@{#465029} (cherry picked from commit eed63750baeef51b69420b637d4e42c6be2c446d) Review-Url: https://codereview.chromium.org/2840623002 . Cr-Commit-Position: refs/branch-heads/3029@{#766} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/77b32554d91e8fd5a1f093038926d262a4bda5a0/ui/views/bubble/bubble_frame_view.cc [modify] https://crrev.com/77b32554d91e8fd5a1f093038926d262a4bda5a0/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/77b32554d91e8fd5a1f093038926d262a4bda5a0/ui/views/window/dialog_delegate_unittest.cc
,
Apr 24 2017
,
May 2 2017
Verified the issue on Ubuntu 14.04 using chrome version #58.0.3029.96 as per the comment #0 and #3. Observed that while clicking on the center of the close button(x) closed the overlay as expected. Hence, the fix is working as expected. Attaching screen cast for reference Hence, adding the verified labels. Thanks...!!
,
May 8 2017
Verified on ChromeOS 9460.30.0, 59.0.3071.41 |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by ajha@chromium.org
, Apr 7 2017Status: Untriaged (was: Unconfirmed)