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

VERIFIED FIXED in Firefox -esr60

Status

()

VERIFIED FIXED
9 months ago
14 days ago

People

(Reporter: alice0775, Assigned: emilio)

Tracking

({regression})

59 Branch
mozilla63
x86_64
Windows 10
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr60 verified, firefox61 verified, firefox62+ verified, firefox63 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
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)
Flags: needinfo?(emilio)

Comment 1

9 months ago
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.
See Also: → bug 1457085
(Assignee)

Comment 2

9 months ago
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.
Assignee: nobody → emilio
See Also: → bug 1471531
(Assignee)

Comment 3

9 months ago
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.
(Assignee)

Comment 4

9 months ago
Am stupid. Found it.
Flags: needinfo?(emilio)
(Assignee)

Updated

9 months ago
Attachment #8988537 - Flags: review?(bugs)
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
Attachment #8988537 - Flags: review+

Updated

9 months ago
Attachment #8988537 - Flags: review?(bugs)
Do we need this on branches?

Comment 8

9 months ago
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 :-(
(Assignee)

Comment 9

9 months ago
(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

9 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/6c5c7f763b65
Fix logic in nsXULPopupManager::ShouldConsumeOnMouseWheelEvent. r=smaug

Comment 11

9 months ago
(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

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6c5c7f763b65
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Blocks: 1470777
Considered Bug 1470777, how hard would it be to uplift this?
(Assignee)

Comment 14

9 months ago
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
Attachment #8988537 - Flags: approval-mozilla-release?
Attachment #8988537 - Flags: approval-mozilla-beta?
status-firefox61: wontfix → fix-optional
No longer blocks: 1470777
Duplicate of this bug: 1470777
tracking-firefox62: --- → +
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.
Attachment #8988537 - Flags: approval-mozilla-release?
Attachment #8988537 - Flags: approval-mozilla-release+
Attachment #8988537 - Flags: approval-mozilla-beta?
Attachment #8988537 - Flags: approval-mozilla-beta+

Comment 17

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/7765ed93426c
status-firefox62: fix-optional → fixed

Comment 18

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/dabf33a256d5
status-firefox61: fix-optional → fixed
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.
Status: RESOLVED → VERIFIED
status-firefox61: fixed → verified
status-firefox62: fixed → verified
status-firefox63: fixed → verified
Flags: qe-verify+
Flags: in-qa-testsuite?
Flags: in-qa-testsuite? → in-qa-testsuite+
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.
Attachment #8988537 - Flags: approval-mozilla-esr60+
This bug is no longer reproducible, also on 60.2.0esr (20180830204350)under Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64.
status-firefox-esr60: fixed → verified
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1471531
Component: Event Handling → User events and focus handling
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.