546071 - Loading Youtube in overflow:hidden div causes 1-pixel jog

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Amy L]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_2; en-us) AppleWebKit/531.21.8 (KHTML, like Gecko) Version/4.0.4 Safari/531.21.10
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6

In the Mac version of Firefox 3.6 only (not 3.5.x or before) including YouTube videos (or any Flash) seems to cause a weird rendering error where the area OVER the overflow:hidden DIV is shifted.

See the error:
http://sequencemediaworks.com/etc/firefox_3_6/youtube.html

But the overflow:hidden doesn't always cause this problem:
http://sequencemediaworks.com/etc/firefox_3_6/ok.html

And previous versions of Firefox don't have this problem. Safari doesn't have this problem.

Reproducible: Always

Steps to Reproduce:
1. Create an HTML page with an overflow:hidden DIV, put inside it a DIV with a YouTube video.

Actual Results:  
The graphics above the overflow:hidden DIV are shifted to the right by 1 pixel.

Expected Results:  
No pixel jog.

Comment 1

[Amy L]

New data point:

If the window.innerWidth reports an ODD number then the problems does NOT show up. If the window.innerWidth is EVEN then the problem shows up...

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: reduced testcase

This testcase requires the test plugin (application/x-test).

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

We need to compute the "view to widget offset" for the plugin's view. It doesn't work properly as-is because the widget for the plugin isn't managed by the view (the view's mWindow is null).

Comment 4

[Mats Palmgren (inactive)]

Comment on attachment 428733 [details] [diff] [review]
fix

Do we need to bump the IID for nsIView?  Adding non-virtual methods
isn't a problem, but I worry about adding mViewToWidgetOffset before
mVFlags which changes its offset. nsIView::GetZIndex() is inline
and uses mVFlags which would require recompiling a consumer.

r=mats if that isn't a problem

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'll bump it.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I landed this, but the reftest failed on all platforms so I backed it out.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm, the reftest passes when I push this patch to the try server. I'll try relanding.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/b3b635d4167d

Relanded. The test failed again, but the test failure seems to be because the test plugin does not load on any of the tests on Tinderbox. haveTestPlugin is in fact false on Tinderbox, so the various plugin tests pass since they're conditioned on haveTestPlugin. I made this test be conditioned on haveTestPlugin too, and filed bug 552365 about the missing test plugin.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually, why do we have haveTestPlugin? Mac, Windows and X have it. Shouldn't we just require that if your platform wants to pass tests, you have a test plugin?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

And definitely, if Mac, Windows or X can't load the test plugin, something should go red.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/2de42ef0e5a9 was from back when we'd just started using the test plugin.  (Was it not present on all platforms then?)  Sounds like it makes sense to get rid of haveTestPlugin.