740341 - panning should not be allowed on a scrolling="no" iframe

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Cristian Nicolae (:xti)]

Firefox 14.0a1 (2012-03-28)
Device: Samsung Galaxy S
OS: Android 2.2

Steps to reproduce:
1. Open Fennec
2. Browse to http://www.browserscope.org/
3. Tap on "How does your browser compare?" > "Go to the Security Tests"
4. Pan the page around
5. Zoom the page in and out
6. Rotate the device to landscape and repeat steps 4-5

Expected result:
No zooming, panning or rendering issues are noticed after performing all reproducing steps.

Actual result:
Zooming is not working correctly and if the page is panned, just the scrollbars are moving accordingly to the pan orientations. Please see the following video: http://youtu.be/aqaGs62tvfk?hd=1

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

possible dup of bug 722657?

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

iframe code has  scrolling="no"  you shouldn't be able to scroll.  I can zoom in on the Galaxy S II.

I think the bug should be more of "scroll bar should not move after zooming and panning on a scrolling="no" iframe"

I personally don't think it's blocking fennec.

Comment 3

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

We should implement support for scrolling="no" and prevent panning on those iframes.

Comment 4

[Martijn Wargers (dead)]

This can also be reproduced in FirefoxOS, testcase here: http://people.mozilla.org/~mwargers/tests/panning/overflow_hidden_no_panning.html
Do we need to create a separate bug for b2g?

Comment 5

[Martijn Wargers (dead)]

I guess this is somewhere checked here? http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/Axis.cpp#365

Comment 6

[Botond Ballo [:botond]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #4)
> This can also be reproduced in FirefoxOS, testcase here:
> http://people.mozilla.org/~mwargers/tests/panning/overflow_hidden_no_panning.
> html
> Do we need to create a separate bug for b2g?

No, I believe this is a Gecko issue.

(In reply to Martijn Wargers [:mwargers] (QA) from comment #5)
> I guess this is somewhere checked here?
> http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/Axis.cpp#365

For most iframes, we don't need to create an APZC (and thus Axis objects) at all if it's scrolling=no. That change needs to be made in nsGfxScrollFrame::WantAsyncScroll [1]. It currently handles overflow:hidden, but it should handle scrolling=no as well.

If the iframe is a remote iframe, it still needs to get an APZC, but we need to truncate its scrollable rect to get APZ to believe it's not scrollable. That happens in nsLayoutUtils::CalculateScrollableRectForFrame(). Again, it currently handles overflow:hidden but not scrolling=no. (As a result of that truncation, Axis::GetPageLength() will return a smaller value and Axis::CanScroll() will return false, but those function don't need to be changed.)

[1] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp?rev=d3b41b0ead7d#1087
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#7608

Comment 7

[Leonard Camacho [:lcamacho]]

From irc

> mstange: WantAsyncScroll() uses GetScrollbarStylesFromFrame(), which should already be handling scrolling="no" (through HandleScrollPref, which respects nsIScrollable::Scrollbar_Never, which is set in response to NS_STYLE_FRAME_NOSCROLL, see  https://mxr.mozilla.org/mozilla-central/source/dom/html/nsGenericHTMLFrameElement.cpp#176 )

Comment 8

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

This appears to be fixed in nightly with APZ. We should retest once APZ is riding the trains on Fennec.

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

I agree this looks fixed on nightly.

Comment 10

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

Fixed with APZ