Issue metadata
Sign in to add a comment
|
Regression: Lower half of Tabstrip Above Omnibox Non-Interactive While Omnibox Suggestions Are Displayed
Reported by
khushal....@etouch.net,
Jul 18
|
||||||||||||||||||||||
Issue descriptionChrome Version : 69.0.3495.0 (Official Build) Revision bc6daf0166d18e090b7026ceae6c5d4f448d082b-refs/branch-heads/3495@{#1} (64 bit) OS: Mac (10.12.6, 10.13.1, 10.13.6, 10.14) Pre-condition: Enable the flag "Use Views browser windows instead of Cocoa." from chrome://flags/ page. UPDATED STEPS TO REPRODUCE - 2018 Jul 18 11:20 PDT 1) Fire up Chrome with a few tabs 2) Bring up the Omnibox suggestions window by typing into the omnibox 3) Mouse hover over the tabstrip. Note the lower halves of tabs do not receive hover effects and are not interactive. Steps to reproduce: 1. Launch chrome and type anything in Omnibox such that suggestion list appears. 2. Now try to add New tab by clicking the lower half of New Tab (+) icon in TabStrip and Observe. Actual Result: 1. Lower half of New Tab (+) icon in TabStrip does not respond on click. 2. Focus does not appear on lower half of New Tab (+) icon on mouse hover. Expected Result: 1. The whole New Tab (+) icon should respond properly on click. 2. Focus should appear on whole New Tab (+) icon on mouse hover. This is a Regression issue seen from 'M-69' and providing the bisect info below: Good Build: 69.0.3480.0 (Revision: 572082) Bad Build: 69.0.3481.0 (Revision: 572437) You are probably looking for a change made after 572208 (known good), but no later than 572209 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/96eb702eb19acebd1f792436749e3021e0b8f54c..1be226e1a5668c6c05627afb95383fb259d51ee2 Suspect: https://chromium.googlesource.com/chromium/src/+/1be226e1a5668c6c05627afb95383fb259d51ee2 @robliao: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. NOTE: 1. Issue is also seen on M-69 Dev (build #69.0.3493.3). 2. New tab (+) icon responds properly on click on Win (7, 8, 8.1, 10) & Linux (14.04 LTS) OS. Kindly refer the attached screen-cast. Thank You..!!
,
Jul 18
This is maybe the area of an (invisible) shadow on top of the Result List?
,
Jul 18
Marking Untriaged for MacViews triage coverage.
,
Jul 18
"RBS" as this is P1 blocking MacViews launch.
,
Jul 18
,
Jul 18
Updating the title as this goes beyond the new tab button. The lower half of the tabstrip above the Omnibox suggestions does not respond to mouse hover while Omnibox suggestions are displayed. Sending this to avi@ as he's recently looking in this area.
,
Jul 18
,
Jul 19
Checked out of band - avi@ is currently working this.
,
Jul 19
The shadow is drawn by a BubbleBorder. Borders don't get hit tests or events. Still investigating.
,
Jul 19
Correct shadow behavior has three parts. 1. Clicks should fall through to the underlying widget. 2. The underlying widget should track the mouse and highlight correspondingly. 3. Clicks through the bottom shadow should dismiss the popup. #1 is easy; I have a patch. #2 is more difficult. RoundedOmniboxResultsFrame is not getting mouse moved events. I hope this is simple. #3 is very difficult. RoundedOmniboxResultsFrame doesn't know directly about the omnibox, and OmniboxView::CloseOmniboxPopup() is the wrong layer. I'd live with being able to fix just #1 and #2.
,
Jul 21
,
Jul 23
For #3, I think GetWidget()->Close() in RoundedOmniboxResultsFrame will "work". It might not animate, but that may be a minor issue. For #1/2.. using the stuff in TopBackgroundView to repost events in RoundedOmniboxResultsFrame as well might do it...
,
Jul 23
I have a patch for 1/2, so I'm landing and will get the merge going.
,
Jul 23
For #3, there is an undocumented property on CALayers (allowsHitTesting) that could be useful here if plumbed through. It can signal the window server to make a layer transparent to mouse events even if it has content, including for the purpose of window hit testing.
,
Jul 23
Re c14, sweet! Can we spin that off as a TODO in a new bug? I'm gonna close this one when I get this fix merged.
,
Jul 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af7dcaaf7c0cb2f0f3c75626f7acd368bf98287c commit af7dcaaf7c0cb2f0f3c75626f7acd368bf98287c Author: Avi Drissman <avi@chromium.org> Date: Mon Jul 23 22:18:47 2018 Forward events to the tab strip. The shadow around the omnibox results popup eats mouse events. Manually forward events occurring in the shadow area to the underlying widget to fix the tab strip. BUG= 864963 , 838667 Change-Id: I5c6499f5d024863dad611f8670fb12e1a01ff073 Reviewed-on: https://chromium-review.googlesource.com/1144225 Commit-Queue: Avi Drissman <avi@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#577293} [modify] https://crrev.com/af7dcaaf7c0cb2f0f3c75626f7acd368bf98287c/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc [modify] https://crrev.com/af7dcaaf7c0cb2f0f3c75626f7acd368bf98287c/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.h
,
Jul 23
,
Jul 24
Update: Rechecked the above issue on latest canary version 70.0.3501.0 on Mac (10.12.6, 10.13.1, 10.13.6, 10.14) and the issue is found Fixed. Hence, adding the respective labels. Please refer the attached screen-cast. Thank You..!!
,
Jul 24
Approving merge to M69 branch 3497 based on comment #18. Please merge ASAP so we can take it in for tomorrow's M69 Dev release. Thank you.
,
Jul 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f37fee31d526b2197d5b88590474391bc0e717f4 commit f37fee31d526b2197d5b88590474391bc0e717f4 Author: Avi Drissman <avi@chromium.org> Date: Tue Jul 24 19:57:56 2018 Forward events to the tab strip. The shadow around the omnibox results popup eats mouse events. Manually forward events occurring in the shadow area to the underlying widget to fix the tab strip. BUG= 864963 , 838667 TBR=avi@chromium.org (cherry picked from commit af7dcaaf7c0cb2f0f3c75626f7acd368bf98287c) Change-Id: I5c6499f5d024863dad611f8670fb12e1a01ff073 Reviewed-on: https://chromium-review.googlesource.com/1144225 Commit-Queue: Avi Drissman <avi@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#577293} Reviewed-on: https://chromium-review.googlesource.com/1148792 Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#50} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/f37fee31d526b2197d5b88590474391bc0e717f4/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc [modify] https://crrev.com/f37fee31d526b2197d5b88590474391bc0e717f4/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.h
,
Jul 24
Can this be marked as fixed if nothing else is pending?
,
Jul 24
Ah, yes.
,
Jul 25
Update: Rechecked the above issue on latest Dev version 69.0.3497.12 on Mac (10.12.6, 10.13.1, 10.13.6, 10.14) and the issue is found Fixed. Hence, adding the respective labels. Please refer the attached screen-cast. Thank You..!! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rbasuvula@chromium.org
, Jul 18