1471415 - No longer able to scroll content area with mouse wheel while arrow panel pop-upped

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Alice0775 White]

Reproducible: always

Steps To Reproduce:
1. Open any long page (e.g, https://www.mozilla.org/en-US/firefox/ )
2. Click on identify box  / Star icon / Library icon / Menu icon
3. Try scroll content area with mouse while

Actual results:
unable to scroll content area with mouse wheel

Expected Results:
able to scroll


Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9096413092d05a53cd5da66ffe8ed2dfd892fa6b&tochange=fceda645f5e3f6cb4f429d5c8efcdeb5a6913e40


Regressed by: fceda645f5e3	Emilio Cobos Álvarez — Bug 1423990: Move the last few attribute-related methods outside of nsIContent. r=bz


:emilio,
Your patch seems to cause the regression, can you please look into this?


(FYI, notification arrow panel such as webRTC-shareDevices-notification are not affected)

Comment 1

[Jorg K (CEST = GMT+2)]

Maybe also related to bug 1457085? There the event gets shipped to the outer list instead of the inner. I believe the scroll feature is quite important for TB users.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Welp... I'll take a look, yeah, thanks Alice.

(In reply to Jorg K (GMT+2) from comment #1)
> Maybe also related to bug 1457085? There the event gets shipped to the outer
> list instead of the inner. I believe the scroll feature is quite important
> for TB users.

As a TB user I agree :-). We'll see whether it's related or not.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yesterday I audited the patch carefully and I couldn't find anything, I'm checking out that revision and reverting, then partially re-applying to see which change introduces it.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Am stupid. Found it.

Comment 5

[Phabricator Automation]

Attachment: Bug 1471415: Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

It was accidentally toggled in bug 1423990.

Comment 6

[Phabricator Automation]

Comment on attachment 8988537 [details]
Bug 1471415: Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

Olli Pettay [:smaug] has approved the revision.

https://phabricator.services.mozilla.com/D1863

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Do we need this on branches?

Comment 8

[Jorg K (CEST = GMT+2)]

Sadly it doesn't fix bug 1457085. There instead of scrolling the autocomplete popup list, the underlying list of recipients is scrolled, so clearly the wrong widget processes the scroll event :-(

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(In reply to Jorg K (GMT+2) from comment #8)
> Sadly it doesn't fix bug 1457085. There instead of scrolling the
> autocomplete popup list, the underlying list of recipients is scrolled, so
> clearly the wrong widget processes the scroll event :-(

Was that regressed by the same patch? That looks like a way more recent regression according to the regression range there.

(In reply to Olli Pettay [:smaug] from comment #7)
> Do we need this on branches?

Presumably, yeah, though nobody noticed until now...

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c5c7f763b65
Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

Comment 11

[Jorg K (CEST = GMT+2)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #9)
> Was that regressed by the same patch?
No, it wasn't. It was regressed by this line:
https://hg.mozilla.org/mozilla-central/rev/63c152ad62b0#l4.12
when the autocomplete popup was switched from a tree to a list in bug 1427366. As a consequence two lists are now interacting badly: The popup and the list of recipients behind it. This switch exposed an ancient XUL bug that the event gets delivered to the wrong consumer, or the wrong XUL widget consumes it. Sadly TB seems to be the only Gecko application to use "list on top of list", so it's up to us to fix the underlying toolbox :-( - I was hoping that perhaps the fix to "something goes to the wrong consumer" in this bug might fix the issue, but that was wishful thinking.

Comment 12

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/6c5c7f763b65

Comment 13

[Marco Bonardo [:mak]]

Considered Bug 1470777, how hard would it be to uplift this?

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8988537 [details]
Bug 1471415: Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

Approval Request Comment
[Feature/Bug causing the regression]: bug 1423990
[User impact if declined]: See the description of this bug, bug 1470777, bug 1471531
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: see STR of this bug.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: one-character fix that restores the logic from pre-bug 1471531
[String changes made/needed]: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8988537 [details]
Bug 1471415: Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

Trivial logic error fix, approving for 62.0b5 and 61.0.1.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7765ed93426c

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/dabf33a256d5

Comment 19

[Anca Soncutean, Desktop QA]

I've reproduced this issue on Firefox 63.0a1 (2018-06-26) under Windows 10 x64. 
This behavior no longer occurs on Firefox 63.0a1 (2018-07-04), Firefox 62.0b5 and Firefox 61.0.1 under Windows 10 x64, Ubuntu 16.04 x32 and macOS 10.12.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8988537 [details]
Bug 1471415: Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

This appears to be working well on release, taking for ESR 60.2 also.

Comment 21

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/bcb4837d51e8

Comment 22

[Anca Soncutean, Desktop QA]

This bug is no longer reproducible, also on 60.2.0esr (20180830204350)under Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64.