1694898 - Scroll not working in Firefox 86 macOS in Extension Popover

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[winlinx]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_1_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.150 Safari/537.36

Steps to reproduce:

In Firefox 86 (only in macOS) for some reason such simple webextension have a bug. Here what happening:

• browser_action popover have a div with display: flex with two child div's
• each of this divs have a overflow: auto`
• scroll works for first div, but doesn't for second

Here reproducable repo: https://github.com/exentrich/firefox-86-macos-webext-overflow-bug

Actual results:

Scroll doesn't work for second div

Expected results:

Scroll should work for any div

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Cocoa' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Alex Cornestean]

Hello,

I’ve managed to reproduce the issue using the attached extension on the latest Nightly (88.0a1/20210228215216), Beta (87.0b4/20210228185859) and Release (86.0/20210222142601) under macOS 11.1.

Also tested on Windows 10 x64, with the mentioned browser versions, however, the issue could not be reproduced there.

Comment 3

[Tomislav Jovanovic :zombie]

Alex, can you please find the regression range for this, it's likely some low-level change since it's OSX only.

Comment 4

[Alex Cornestean]

Performed the bisection and got the following:

Last good revision: 8c8bbc19e7f90f621bc92f805db1fddcdad605b4
First bad revision: d8bd960f9b82ca8d9ef340ae488673b6f0c0669e
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8c8bbc19e7f90f621bc92f805db1fddcdad605b4&tochange=d8bd960f9b82ca8d9ef340ae488673b6f0c0669e

The regressor seems to be: https://bugzilla.mozilla.org/show_bug.cgi?id=1493208 (Re-enable APZ in popups with remote content)

Comment 5

[Hiroyuki Ikezoe (:hiro)]

So, this happens only on Mac 11? not on 10.x? Also I am wondering whether WebRender is enabled or not.

Comment 6

[Timothy Nikkel (:tnikkel)]

Happens on 10.15 as well for me (with webrender, later I tested and it happened without webrender too).

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Thank you Timothy! Unfortunately it's working with WebRender on my mac book, it's Mac 10.14.6.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Confirmed without WebRender.

Comment 9

[Timothy Nikkel (:tnikkel)]

Reproduces without webrender for me too (reproduced about with webrender). I get the same regression range as comment 4 with or without webrender.

Comment 10

[Timothy Nikkel (:tnikkel)]

I can reproduce with and without webrender. Webrender does not seem to change anything with regards to this bug for me.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Dropping "[non WebRender]" from the bug title since Timothy can reproduce the issue both on WebRender and non WebRender.

Comment 12

[Timothy Nikkel (:tnikkel)]

If I force the apz.popups.enabled pref to true I get this regression range

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a859a4b2257e6c61f7c2aab456cf551df856bd95&tochange=55d91695f4bb951c224005155baef054a786c1f7

not sure what might have caused this in there.

Comment 13

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #12)

If I force the apz.popups.enabled pref to true I get this regression range

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a859a4b2257e6c61f7c2aab456cf551df856bd95&tochange=55d91695f4bb951c224005155baef054a786c1f7

not sure what might have caused this in there.

Probably bug 1385403.

Comment 14

[Timothy Nikkel (:tnikkel)]

The weird thing is that if you do the scroll gesture on the track pad the scrollbar fades in, so it's targeting the right scroll frame. And if you get the scroll frame scrolling via another means (keyboard, some scrollbar interactions) then you can scroll via the trackpad.

Comment 15

[Timothy Nikkel (:tnikkel)]

Turning on apz.wr.activate_all_scroll_frames does seem to fix it. So we're failing to set a displayport somehow maybe?

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Another interesting fact I noticed is that enabling fission fixes this issue on WebRender. Fission is the reason why I couldn't reproduce this issue with WebRender.

Comment 17

[Timothy Nikkel (:tnikkel)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #16)

Another interesting fact I noticed is that enabling fission fixes this issue on WebRender. Fission is the reason why I couldn't reproduce this issue with WebRender.

That's because the pref apz.wr.activate_all_scroll_frames_when_fission is true by default.

Comment 18

[Timothy Nikkel (:tnikkel)]

The first scrollframe probably gets a display port via MaybeCreateDisplayPortInFirstScrollFrameEncountered and then we don't get any PrepareForSetTargetAPZCNotification calls so the second one never gets a display port. The scrollbars becoming visible is probably a content side effect, so content hittest is working.

Comment 19

[winlinx]

Guys do you know any workaround? This bug makes our extension unusable.

Comment 20

[Hiroyuki Ikezoe (:hiro)]

Setting apz.popups.enabled to false is a workaround. I am sorry for the inconvenience.

Comment 21

[winlinx]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #20)

Setting apz.popups.enabled to false is a workaround. I am sorry for the inconvenience.

Thanks. But I mean in terms of HTML/CSS/JS. Without modifying Firefox flags, is it possible?

Comment 22

[Kartikaya Gupta (email:[e-mail])]

You could try setting will-change: scroll-position in the CSS for the element that has overflow: auto, that might help by causing the displayport to get set.

Comment 23

[winlinx]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #22)

You could try setting will-change: scroll-position in the CSS for the element that has overflow: auto, that might help by causing the displayport to get set.

Doesn't work unfortunately :( Maybe anything else?

Comment 25

[Hiroyuki Ikezoe (:hiro)]

inputBlockIds for mouse wheel events in BrowserParent::SendMouseWheelEvent are always zero, that's a problem. And the reason why it's zero is this AsyncPanZoomEnabled() returns false for some reasons. It's a widget thing.

Comment 26

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1694898 - Override AsyncPanZoomEnabled in nsCocoaWindow to delegate it to nsChildView in the case of popups. r?mstange!,botond!

Comment 27

[Hiroyuki Ikezoe (:hiro)]

D109572 should fix this issue, I will add a mochitest in the change.

Comment 28

[Timothy Nikkel (:tnikkel)]

(In reply to winlinx from comment #23)

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #22)

You could try setting will-change: scroll-position in the CSS for the element that has overflow: auto, that might help by causing the displayport to get set.

Doesn't work unfortunately :( Maybe anything else?

Try calling scrollBy on the scrollable elements?

Comment 29

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #22)

You could try setting will-change: scroll-position in the CSS for the element that has overflow: auto, that might help by causing the displayport to get set.

I filed bug 1700804 to set a display port for will-change: scroll because that seems like a good idea. That bug isn't going to help here, but I'm linking it here for reference.

Comment 30

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1694898 - Factor out flushApzRepaintsInPopup. r?botond

We'd like to use the function in a new mochitest.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eba1a4970a99
Factor out flushApzRepaintsInPopup. r=botond
https://hg.mozilla.org/integration/autoland/rev/b957e5159972
Override AsyncPanZoomEnabled in nsCocoaWindow to delegate it to nsChildView in the case of popups. r=mstange,botond

Comment 32

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/eba1a4970a99
https://hg.mozilla.org/mozilla-central/rev/b957e5159972

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:hiro, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 34

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 9211175 [details]
Bug 1694898 - Override AsyncPanZoomEnabled in nsCocoaWindow to delegate it to nsChildView in the case of popups. r?mstange!,botond!

Beta/Release Uplift Approval Request

• User impact if declined: Contents cannot be scrolled
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is pretty straight forward, and it has a null check.
• String changes made/needed: none

Comment 35

[winlinx]

Does it possible to see this fix anytime soon? Firefox 89 is way to long to wait, like more than month.

Comment 36

[Timothy Nikkel (:tnikkel)]

It will likely get uplifted to 88.

Did you try the suggested workaround in comment 28 ?

Comment 37

[winlinx]

(In reply to Timothy Nikkel (:tnikkel) from comment #36)

It will likely get uplifted to 88.

Did you try the suggested workaround in comment 28 ?

Yep, no success. scrollBy is working, but it doesn't fix scroll by mouse wheel / trackpad

Comment 38

[raja]

(In reply to Timothy Nikkel (:tnikkel) from comment #36)

It will likely get uplifted to 88.

Did you try the suggested workaround in comment 28 ?

No success with that workaround on my end either. Firefox 89 is definitely too long a wait for this

Comment 39

[winlinx]

It's not a feature request, it's a bug, like critical one for my opinion. Please escalade it as possible

Comment 40

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9211175 [details]
Bug 1694898 - Override AsyncPanZoomEnabled in nsCocoaWindow to delegate it to nsChildView in the case of popups. r?mstange!,botond!

Approved for 88.0b5.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/58ba360c67fd
https://hg.mozilla.org/releases/mozilla-beta/rev/3bd6fe4e28a2

Comment 42

[Alex Cornestean]

Hello,

The issue is fixed on the latest Beta (88.0b5/20210330185720) and Nightly (89.0a1/20210330215136). Tested with macOS 11.2.2.

Comment 43

[Hiroyuki Ikezoe (:hiro)]

(In reply to raja from comment #38)

(In reply to Timothy Nikkel (:tnikkel) from comment #36)

It will likely get uplifted to 88.

Did you try the suggested workaround in comment 28 ?

No success with that workaround on my end either. Firefox 89 is definitely too long a wait for this

Hey, raja, I just confirmed that normal scrollBy doesn't fix this, you will need to specify behavior: "smooth", something like this; "element.scrollBy({top: 0, behavior: 'smooth'});". Hope this fixes your case. Thanks!