With apz enabled, you can scroll content by panning a flyout panel

RESOLVED FIXED in Firefox 26

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
Firefox 26
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Assignee

Description

6 years ago
STR:

1) open a scrollable content page
2) open the about flyout
3) try to scroll the about flyout

result: the content page scrolls
Assignee

Comment 1

6 years ago
Posted patch patch v.1 (obsolete) — Splinter Review
Assignee: nobody → jmathies
Assignee

Comment 2

6 years ago
Comment on attachment 796167 [details] [diff] [review]
patch v.1

What this does is filter out everything but the browser in our capture listener in input.js to call preventDefault which kills off apz scrolling. Chatted with mbrubeck on irc about the non-e10s friendly nature of this which he was ok with.
Attachment #796167 - Flags: review?(netzen)
Comment on attachment 796167 [details] [diff] [review]
patch v.1

Review of attachment 796167 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/input.js
@@ +214,5 @@
> +    // Help out chrome ui elements that want input.js vs. apz scrolling: call
> +    // preventDefault when apz is enabled on anything that isn't in the
> +    // browser. Not e10s friendly.
> +    if (APZCObserver.enabled &&
> +        !getBrowser().contentDocument.documentElement.contains(aEvent.target)) {

This could be very inefficient for some pages, let's use .ownerDocument per irc.
Attachment #796167 - Flags: review?(netzen)
Assignee

Comment 4

6 years ago
Posted patch patch v.2 (obsolete) — Splinter Review
Attachment #796167 - Attachment is obsolete: true
Attachment #796184 - Flags: review?(netzen)
Attachment #796184 - Flags: review?(netzen) → review+
Assignee

Comment 5

6 years ago
This is going to need some more work. I think we need to hollow out some of our status checking down in MetroInput when apz is enabled, which also checks the return result.
Assignee

Comment 6

6 years ago
Unfortunately this broke button clicks in the nav bar. :/ Need to investigate further.
Assignee

Updated

6 years ago
Depends on: 913707
Assignee

Comment 7

6 years ago
Comment on attachment 796184 [details] [diff] [review]
patch v.2

This fix is close, however we need to do the cancelling on the first touch move, and there are a couple fixes to dom touch and widget as well.
Attachment #796184 - Flags: review+
Assignee

Updated

6 years ago
Duplicate of this bug: 913059
Assignee

Comment 9

6 years ago
Posted patch widget fix (obsolete) — Splinter Review
Attachment #796184 - Attachment is obsolete: true
Assignee

Comment 10

6 years ago
Posted patch front end (obsolete) — Splinter Review
Assignee

Comment 11

6 years ago
Posted patch front end fix (obsolete) — Splinter Review
Attachment #801224 - Attachment is obsolete: true
Assignee

Updated

6 years ago
Duplicate of this bug: 913707
Assignee

Comment 13

6 years ago
Posted patch widget fixSplinter Review
Attachment #801223 - Attachment is obsolete: true
Attachment #801225 - Attachment is obsolete: true
Attachment #801272 - Flags: review?(tabraldes)
Assignee

Comment 14

6 years ago
Posted patch front end fixSplinter Review
Attachment #801274 - Flags: review?(mbrubeck)
Attachment #801274 - Flags: review?(mbrubeck) → review+
Comment on attachment 801272 [details] [diff] [review]
widget fix

Review of attachment 801272 [details] [diff] [review]:
-----------------------------------------------------------------

Seems fine. Some day it would be nice to pick apart nsPresShell and widget, but I'm sure not signing up for that job :)
Attachment #801272 - Flags: review?(tabraldes) → review+
https://hg.mozilla.org/mozilla-central/rev/f583bcbb5e82
https://hg.mozilla.org/mozilla-central/rev/14eb0d5731d6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.