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

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Pan and Zoom, defect)

Description

[Jim Mathies [:jimm]]

STR:

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

result: the content page scrolls

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 2

[Jim Mathies [:jimm]]

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.

Comment 3

[Brian R. Bondy [:bbondy]]

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.

Comment 4

[Jim Mathies [:jimm]]

Attachment: patch v.2

Comment 5

[Jim Mathies [:jimm]]

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.

Comment 6

[Jim Mathies [:jimm]]

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

Comment 7

[Jim Mathies [:jimm]]

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.

Comment 9

[Jim Mathies [:jimm]]

Attachment: widget fix

Comment 10

[Jim Mathies [:jimm]]

Attachment: front end

Comment 11

[Jim Mathies [:jimm]]

Attachment: front end fix

Comment 13

[Jim Mathies [:jimm]]

Attachment: widget fix

Comment 14

[Jim Mathies [:jimm]]

Attachment: front end fix

Comment 15

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

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 :)

Comment 16

[Jim Mathies [:jimm]]

Thanks!

remote:   https://hg.mozilla.org/integration/fx-team/rev/f583bcbb5e82
remote:   https://hg.mozilla.org/integration/fx-team/rev/14eb0d5731d6

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f583bcbb5e82
https://hg.mozilla.org/mozilla-central/rev/14eb0d5731d6