search

PolymerElements / iron-overlay-behavior

Makes an element an overlay with an optional backdrop
41 stars 71 forks source link

Firefox: Visual glitches are triggered by the scrolling attempt, when the overlays that lock scrolling position are opened. #266

Open helenxywu opened 6 years ago

helenxywu commented 6 years ago

Description

Open overlay that locks scroll position in Firefox. Try scrolling with scrollbar. Details here https://bugzilla.mozilla.org/show_bug.cgi?id=1438514#c5.

Expected outcome

Nothing happens

Actual outcome

Visual glitches are triggered by the scrolling attempt.

Live Demo

See attachment here https://bugzilla.mozilla.org/show_bug.cgi?id=1438514

Browsers Affected

valdrinkoshi commented 6 years ago

This cannot be reproduced in Firefox 58.0.2 for Mac OS 10.13.3 (17D47), will try on windows soon

theres-waldo commented 6 years ago

Note: reproducing this requires a heavy page like Youtube, which keeps the browser's main thread busy enough. It helps (to repro the issue) to open the overlay and try to scroll quickly after loading the page, while some content on the page (e.g. comments) is still being loaded.

valdrinkoshi commented 6 years ago

Thanks @theres-waldo! I've written a simpler repro that creates jank on the main thread, and now I can repro on firefox for mac too: http://jsbin.com/jaqeyo/1/edit?html,output

valdrinkoshi commented 6 years ago

I'll investigate if it's possible to do something about it, but I'm not confident there is much to do. Some additional info on how we lock scrolling in case it helps:

The fact that the main thread is busy makes the scroll position restoration more visible.

Would it be an option to use another action on scroll? iron-overlay-behavior can also close the overlay on scroll for you, using scrollAction = 'cancel', e.g. <iron-dropdown scroll-action="cancel"> http://jsbin.com/nanejed/1/edit?html,output

theres-waldo commented 5 years ago

I'll investigate if it's possible to do something about it, but I'm not confident there is much to do. Some additional info on how we lock scrolling in case it helps:

The suggestion I made in the Firefox bug where this came up, is to lock scrolling by making the page overflow:hidden while the overlay is shown. Has this been considered?

valdrinkoshi commented 5 years ago

@theres-waldo that would cause layout changes on browsers that apply an offset when the scrollbar is shown vs not, e.g. in IE11 toggling overflow:hidden/auto would add/remove 20px to the width if the page is vertically scrollable.

theres-waldo commented 5 years ago

Is that a concern because of the performance impact of the re-layout? Or because the change to the layout might be visually jarring to the user? (I expect a scrollbar-width amount of change is unlikely to change the layout in any significant way, but you can probably construct a page where it does...)

valdrinkoshi commented 5 years ago

Basically, all of the above. This approach was introduced here https://github.com/PolymerElements/iron-dropdown/pull/93

theres-waldo commented 5 years ago

How involved would it be, to use the overflow:hidden approach for some browsers, and the scroll event handler approach on others? I think the issues described in https://github.com/PolymerElements/iron-dropdown/pull/93 are less severe than the effects of using a scroll event handler to adjust the scroll position given Firefox's asynchronous scrolling.

valdrinkoshi commented 5 years ago

I don't think there is a silver bullet for this one. We thought of solutions like adding an element to the dom just to measure the scrollbar width (e.g. https://davidwalsh.name/detect-scrollbar-width), but that causes layout/paint on the whole document, and potentially triggers mutation observers, and it would be leaking implementation details. I'll leave it to the owners (@bicknellr fyi) to decide on this one (I'm not maintaining the repo anymore).

Probably this can be fixed at the app level by settings overflow: hidden when any scroll-blocking dialog/dropdown is opened.