Issue metadata
Sign in to add a comment
|
Files app: Scroll bar can not be dragged by mouse. |
||||||||||||||||||||||
Issue descriptionChrome Version: ToT OS: Chrome OS What steps will reproduce the problem? (1) Open Files app (2) Navigate to a folder which has many files (3) Try to drag scroll bar using mouse What is the expected result? Scroll bar can be dragged What happens instead? Area selection is triggered. I guess this is a regression. yamaguchi@: When we migrated to overlay scrollbar, was it working?
,
Aug 18 2017
This also affects other places such as the directory tree view in the Files app. and the console in the developer tools of the browser. Now I have identified the issue happens after this change. https://chromium-review.googlesource.com/c/613640 Issue 733546 is the relevant issue.
,
Aug 18 2017
chaopeng@: Based on my bisecting result, I am thinking the change on the overlay scrollbar is affecting some apps using scrollable elements. However I am not sure if it's a bug. Will you take a look?
,
Aug 18 2017
,
Aug 18 2017
,
Aug 18 2017
This issue shows without kExcludeOverlayScrollbarSizeForHitTesting, Hit test may skip the scrollable and go to the children of scrollable. So https://chromium-review.googlesource.com/c/613640 is wrong. We should find another approach.
,
Aug 18 2017
Hmm...isn't the issue that we're missing the scrollbar thumb in the hit test? i.e. if the hit test occured outside the scrollbar thumb but still inside the track rect we *should* hit the content and allow selection.
,
Aug 18 2017
Lets work on figuring out the correct solution, but +RBB to make sure we don't ship this.
,
Aug 18 2017
,
Aug 18 2017
The log shows
LayoutBlockFlow (relative positioned) 0x2ba1d3627070 {LIST} at (0,40) size 759x212 id="file-list" class="list"
and
LayoutFlexibleBox (relative positioned) 0x2ba1d3636b48 {LI} at (0,0) size 759x40 id="listitem-1" class="table-row directory"
In the LayoutTree, file-list is listitem-1's parent. But in PaintLayerTree, file-list and listitem-1 in same level. That is why file-list get skiped by fragment correct without kExcludeOverlayScrollbarSizeForHitTesting.
,
Aug 21 2017
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
,
Aug 21 2017
,
Aug 22 2017
FYI, this bug also breaks scrollbars dragging on Tweetdeck. So yea, it's a public bug. If you have a Twitter account, login at https://tweetdeck.twitter.com/ and see. I'm on Windows with the overlay scrollbar flag enabled.
,
Aug 22 2017
Here is the minimize test case from file app. And we can see the log, PL for <list> is not parent of PL for <li>. pdr@ do you have idea about this?
,
Aug 22 2017
Chao, does this case work on Mac's overlay scrollbars?
Philip, do you know why the scroll container (the <list>) is a sibling PaintLayer to the <li> PaintLayers, rather than their parent as I'd expect?
Attached a slightly more minimized case to Chao's above (to make showLayerTree less verbose). Here's the layer tree output:
*layer 0x82f35614010 at (0,0) size 1559x1068 (composited, bounds=at (0,0) size 1559x1068, drawsContent=1)
LayoutView 0x82f35604010 at (0,0) size 1559x1068
positive z-order list(1)
layer 0x82f356140e8 at (0,0) size 1559x1068
LayoutBlockFlow 0x82f35624010 {HTML} at (0,0) size 1559x1068
LayoutFlexibleBox 0x82f35630010 {BODY} at (0,0) size 1559x1068
LayoutFlexibleBox 0x82f35630188 {DIV} at (0,0) size 1559x1068 id="table"
normal flow list(1)
layer 0x82f356141c0 at (0,0) size 800x1068 scrollY 742.00 scrollHeight 3000 (composited, bounds=at (0,0) size 800x1068, drawsContent=1)
LayoutBlockFlow 0x82f35624130 {LIST} at (0,0) size 800x1068 id="list"
positive z-order list(1)
layer 0x82f35614298 at (0,-742) size 800x3000 backgroundClip at (0,0) size 800x1068 clip at (0,0) size 800x1068 (composited, bounds=at (-1,0) size 801x3000, drawsContent=1)
LayoutListItem (relative positioned) 0x82f3563c010 {LI} at (0,0) size 800x3000
LayoutListMarker (anonymous) 0x82f35624250 at (-1,0) size 7x17: bullet
LayoutText 0x82f35644010 {#text} at (22,0) size 42x17
text run at (22,0) width 42: "Item 1"
Presumably the issue is that we're walking the positive-z list before the scroll container's layer which is where the overflow controls are so we return before checking for scrollbars.
,
Aug 22 2017
It also not draggable for Mac
,
Aug 22 2017
The dom order is fairly unrelated to the paint order when positioning is involved. For example:
<!DOCTYPE html>
<style>
.myrel {
position: relative;
background: lightblue;
width: 100px;
height: 100px;
}
.myrelchild {
position: relative;
z-index: -100;
background: lightpink;
width: 200px;
height: 100px;
}
</style>
<div class="myrel">
<div class="myrelchild"></div>
</div>
Will produce a layer tree:
positive z-order list(1)
layer 450080A8 at (0,0) size 1041x116 layerType: background only
negative z-order list(1)
layer 450081D8 at (8,8) size 200x100
LayoutBlockFlow (relative positioned) 450180D8 zI: -100 {DIV} at (0,0) size 200x100 [bgcolor=#FFB6C1] class="myrelchild"
...
positive z-order list(1)
layer 45008308 at (8,8) size 100x100
LayoutBlockFlow (relative positioned) 45018330 {DIV} at (0,0) size 100x100 [bgcolor=#ADD8E6] class="myrel"
The testcase has a lot going on so it's hard to see that this is just stacking context changing paint order. Does that help?
,
Aug 22 2017
Here's an example showing how paint order can be independent of dom order: https://docs.google.com/presentation/d/1ak7YVrJITGXxqQ7tyRbwOuXB1dsLJlfpgC4wP7lykeo/edit#slide=id.g211348a01e_0_754
,
Aug 23 2017
Yeah, I know paint order can vary in surprising ways. I guess I'm just surprised in this case since I'd expect an overflow clipping parent with scrollbars to be higer up in the paint order than its overflow content. i.e. since we're painting scrollbars over the content. So it seems to me that the scrollbars actually _are_ painted below the child content in these cases but we've side-stepped the issue since scrollbars used to always totally occlude the content below - we just apply the clip and then we don't see any ordering issues. This works fine until you want scrollbars to actually show some of the content behind them (visibily, Aura overlay scrollbars look correct today because the scrollbars are drawn by the compositor). This is a bit of a head-scratcher, see https://output.jsbin.com/gukuworaze with overlays turned off. The greenbox is a child of the scroller, but appears on top of the red box which is outside the scroller but draws over the scrollbars. How should an overlay scrollbar behave here? Should the green box be painted over the scrollbars? Should the scrollbars paint overtop of everything (visibly, they do today but not for hit testing)? I don't have a good solution here yet. pdr, any ideas? Do you know how overlay scrollbars are painted in Blink? They appear over everything today when drawn from Blink (see attached image) so we clearly have a special case for painting them that isn't reflected in hit testing. Perhaps we could insert a dummy paint layer that parents a scroller and its children solely for the purpose of hittesting its scrollbars? In the mean time, I think we should revert this patch to fix the regression (there's some related perf regressions too).
,
Aug 23 2017
I do not think this is specified but I've looked at firefox and edge and our behavior and I think we should paint scrollbars just after we paint the border (https://www.w3.org/TR/CSS2/zindex.html). Our behavior is not exactly this ( https://jsbin.com/vikiyam) and I think that it's a bug. Painting just after the border means that scrollbars would paint before negative z-index children. I think it's a bug if hit test order and paint order are different.
,
Aug 23 2017
I filed https://crbug.com/758351 for changing our paint and hit test order slightly to paint after the border. It seems like an easy change and will improve compat but so many scrollbar changes seem easy at the start..
,
Aug 23 2017
Tien-Ren pointed out that things are more complicated and overlay scrollbars which are intentionally painted later to avoid being clipped (more explanation in crbug.com/758351). To the original question in comment #19, is it possible to just special-case hit testing overlay the same way we special-case painting? This seems to be done without an extra paintlayer today.
,
Sep 13 2017
Marking this as fixed since the patch in question was reverted in r496917. Lets track re-fixing the original scrollbar hittesting issue in issue 733546 and issue 758351.
,
Sep 13 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-62; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-62 label, otherwise remove Merge-TBD label. Thanks.
,
Sep 13 2017
No merge required, the patch was reverted before 62 branched. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by yamaguchi@chromium.org
, Aug 18 2017