Closed Bug 943206 Opened 6 years ago Closed 6 years ago

Images and Videos scroll when they shouldn't when in frames with overflow-y: hidden

Categories

(Core :: Layout, defect)

Other
Android
defect
Not set

Tracking

()

RESOLVED INCOMPLETE
Tracking Status
firefox25 --- affected
firefox26 --- affected
firefox27 --- affected
firefox28 --- affected
firefox29 - affected
fennec + ---

People

(Reporter: blassey, Unassigned)

References

()

Details

(Keywords: regression, reproducible)

http://www.pbs.org/newshour/rundown/2013/11/innovative-justice-program-sweeping-the-usa.html

Go to the above page and pan on the map graphic. It'll pan out of its frame. I've seen this on multiple pages recently.
I'm able to pan the map out of the frame on all channels with your test URL, so I don't think this is anything new. This might be a dupe of a known bug. Kats?
tracking-fennec: --- → ?
Flags: needinfo?(bugmail.mozilla)
Keywords: reproducible
This isn't a dupe of any bug I'm aware of. The iframe containing the image has overflow-y: hidden set so we really shouldn't be scrolling it.
Flags: needinfo?(bugmail.mozilla)
Summary: Images scroll when they shouldn't → Images scroll when they shouldn't when in frames with overflow-y: hidden
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> This isn't a dupe of any bug I'm aware of. The iframe containing the image
> has overflow-y: hidden set so we really shouldn't be scrolling it.

Sounds like the sort of thing that should have been fixed by bug 942995, which just landed recently. Does this problem still occur with that fix?
This is still reproducible on today's Nightly (12/02) with the URL in this bug.
(In reply to Botond Ballo [:botond] from comment #3)
> Sounds like the sort of thing that should have been fixed by bug 942995,
> which just landed recently. Does this problem still occur with that fix?

Fennec doesn't use APZC so the codepaths are different.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> (In reply to Botond Ballo [:botond] from comment #3)
> > Sounds like the sort of thing that should have been fixed by bug 942995,
> > which just landed recently. Does this problem still occur with that fix?
> 
> Fennec doesn't use APZC so the codepaths are different.

Oh whoops, I'm so immersed in B2G I didn't realize this was a Fennec bug.
Botond, can you try reproducing with the non-java APZC? if it doesnt' reproduce then just mark this bug as depending on switching over.
Assignee: nobody → botond
tracking-fennec: ? → +
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #7)
> Botond, can you try reproducing with the non-java APZC? if it doesnt'
> reproduce then just mark this bug as depending on switching over.

needinfoing myself so I don't forget
Flags: needinfo?(botond)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #7)
> Botond, can you try reproducing with the non-java APZC? if it doesnt'
> reproduce then just mark this bug as depending on switching over.

I can't even get the image on the example page to fully load on my Samsung GT-I5510M... will try on a better device.
Flags: needinfo?(botond)
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #7)
> Botond, can you try reproducing with the non-java APZC? if it doesnt'
> reproduce then just mark this bug as depending on switching over.

The behaviour does not reproduce with the native APZC. I updated the bug's dependency accordingly.
Assignee: botond → nobody
Depends on: apz-fennec
Duplicate of this bug: 955885
Unsimplified test-case:

http://people.mozilla.org/~atrain/mobile/tests/embed-youtube.html

You can scroll off that poster image on the HTML5 video including the big Play button.

This is reproducible on all channels.
From what I can tell there is not a clear timeline for implementing the native APZC and this bug is an issue with the java one - so can we do something intermediate to fix this when using the java APZC?
Flags: needinfo?(mark.finkle)
Flags: needinfo?(bugmail.mozilla)
Not sure if Mark or Brad are in the best position to prioritize/assign this but since it's affecting more than just images I'd like to nudge up the criticality of this issue.
Flags: needinfo?(blassey.bugs)
Summary: Images scroll when they shouldn't when in frames with overflow-y: hidden → Images and Videos scroll when they shouldn't when in frames with overflow-y: hidden
I'm busy with B2G work at the moment but I can certainly mentor somebody through debugging this. The initial debug steps should be pretty straightforward - for some reason _findScrollableElement in browser.js is finding these things as scrollable when it shouldn't be, so figuring out why that's happening would be a good place to start.
Flags: needinfo?(bugmail.mozilla)
We'd really like to get this fixed as its a pretty bad experience. Kats would be the best assignee, but as he said in comment 15 he is otherwise tasked.
Flags: needinfo?(blassey.bugs)
Whiteboard: [mentor=kats]
Whiteboard: [mentor=kats] → [mentor=kats][lang=js]
Flags: needinfo?(mark.finkle)
OK, well that's enough to remove this from tracking consideration - constrained resources and all - but will put out a call for contributions and if there's a low risk fix at some point we can get it uplifted as high as possible.
I can reproduce this on desktop (Mac) on today's nightly.
Duplicate of this bug: 966790
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #18)
> I can reproduce this on desktop (Mac) on today's nightly.

To be clear, this regressed recently on Mac, where as it regressed a few months ago on Android.
nominating for tracking 29 because this regressed on Mac in that 29 (or perhaps 28, we noticed it in 29). Looking for a regression window to confirm.
Component: Graphics, Panning and Zooming → Graphics: Layers
Product: Firefox for Android → Core
Also moving this to Core->Graphics::Layers. Kats, is that the right place for core pan/zoom bugs?
Flags: needinfo?(bugmail.mozilla)
If it were an APZC issue then Core->Panning and Zooming would be the right component. But if it's happening on Mac (where APZ is disabled) then DOM/CSS/Layout seems more appropriate.

I looked at it quickly though to see if I could make an initial diagnosis. It looks like on Firefox, the body of the iframe containing the graphic is 645 pixels tall, and therefore scrollable because it exceeds the clientHeight of 389 pixels. On Chrome the body is 389 pixels tall. In both browsers the div just inside the body is 645 pixels tall, and I see no style set on the body element itself which would explain why it's shorter in Chrome than it is in Firefox. So actually to me this looks like a bug in Chrome, but I haven't checked other browsers to see what they do.

The only other thing I saw that might be relevant is that the <iframe> element itself has overflow-y: hidden set on it. I don't know what the specs say about setting overflow properties on iframe elements, and whether they're supposed to propagate down into the body of the subdocument. If that is the case then this is our bug (probably CSS). So I'm gonna move it there for people to look at.
Component: Graphics: Layers → DOM: CSS Object Model
Flags: needinfo?(bugmail.mozilla)
Whiteboard: [mentor=kats][lang=js] → [happens on mac, see comment 18 onwards]
> On Chrome the body is 389 pixels tall.

Is the document in the iframe in quirks mode, by any chance?

> I don't know what the specs say about setting overflow properties on iframe elements,

Nothing whatsoever that would affect the content.  We used to propagate them into the iframe until bug 943249 aligned us with the spec.  But this bug claims to be reproducible back to Firefox 25, so it's possible that something else is going on here instead of, or in addition to, the overflow propagation bits?  Or are people just mixing up multiple distinct issues in a single bug?
Component: DOM: CSS Object Model → Layout
(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #24)
> > On Chrome the body is 389 pixels tall.
> 
> Is the document in the iframe in quirks mode, by any chance?

In Firefox on OS X the document in the iframe is in standards mode. (At least, document.compatMode returns "CSS1Compat").

> > I don't know what the specs say about setting overflow properties on iframe elements,
> 
> Nothing whatsoever that would affect the content.  We used to propagate them
> into the iframe until bug 943249 aligned us with the spec.  But this bug
> claims to be reproducible back to Firefox 25, so it's possible that
> something else is going on here instead of, or in addition to, the overflow
> propagation bits?  Or are people just mixing up multiple distinct issues in
> a single bug?

Yes, people are mixing up issues in a single bug. The bug was original filed for Fennec (and that's what the "affected" flags indicate) but then got silently repurposed for Mac OS X, which only regressed recently (see comment 18, but pending regressionwindow-wanted). It's quite likely that this was regressed by bug 943249 as the timeframe fits.

Based on this I think this is not a bug in our code but a tech evangelism issue, at least for Firefox on Desktop. The Fennec situation may be different since it was happening before bug 943249 landed.
For the record, bz and I looked at this a bit further yesterday on IRC and discovered that the <div> inside the body of the iframe has a "zoom" property set on it, which accounts for the difference. We (I guess) don't support the zoom property, so they have a fallback that uses a scale transform instead. However zoom affects the layout height and the scale transform does not, which explains the difference in the height of the body between Chrome and Firefox.

Presumably if we support zoom this will problem will go away but otherwise it's a TE bug.
Based on feedback in Comment 26 I don't think we need to track this.
Because this bug is confusing and the test case used to file this bug is actually a TE bug, I filed bug 988991 specifically for the original Fennec problem using Aaron's test case.
And again, because this bug is confusing, I filed bug 989060 for the tech evangelism issue for the website in comment 0.

That covers all the issues in this bug so I'm going to close it out. If there are additional websites that are displaying this behaviour on Mac, please file another bug. If there are additional sites that display this behaviour on Fennec that are NOT fixed by the patch I put on bug 911574, then please file another bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
Whiteboard: [happens on mac, see comment 18 onwards]
You need to log in before you can comment on or make changes to this bug.