520436 - Padding for embedded YouTube videos causes clipped/distorted controls and video content

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: screenshot

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091003 Minefield/3.7a1pre ID:20091003031247

With the fix on bug 508495 the video content is nearly shown perfect on all platforms. Only on OS X I get some static pixels on the right side now which are not getting updated. It's really only visible on OS X and works fine on Windows and Linux. Also Shiretoko doesn't show it. See the screenshot.

Steps:
1. Load the page from the URL field
2. Start the video

Comment 1

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

A small testcase would be useful especially if you can come up with one that doesn't require dynamic changes in plugin content...

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Dynamic testcase

That's a short reduction for the given web page. It shows that the padding is causing this problem. It's applied to the object and embedded element. It's even more than only a couple of pixels now. The controller bar gets distorted too.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Similar artifacts are shown in video content with Namoroka. So it has to be a regression from bug 339548 directly.

Comment 4

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

Attachment: fix

NPN_InvalidateRect's rectangle parameter is relative to the top-left of the plugin content, so we need to adjust it to make it relative to the element's border-box.

To make this testable I've added a setColor method to the test plugin that invalidates the plugin's area. Then it's possible to write invalidation reftests using the test plugin.

Comment 5

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

Checked in:
http://hg.mozilla.org/mozilla-central/rev/9fabec8d72a6

Test failed on Windows/Linux, so I disabled it:
http://hg.mozilla.org/mozilla-central/rev/fa363abd896e

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Roc, can we call it fixed?

Comment 7

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

I'm holding it open to investigate the test failure.

Comment 8

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

Hmm, reftest seems to pass locally and on try servers. I will try to reland.

Comment 9

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

Attachment: make reftest timeout messages a bit more useful

As well as reenabling the test, this patch makes reftest.js output slightly more specific messages when there is a post-onload timeout.

Comment 10

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

I'll mark this fixed to avoid confusion.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091027 Minefield/3.7a1pre ID:20091027031433

Comment 12

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

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e9c76933d003

Comment 13

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

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/35fba3e81f1d

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b2pre) Gecko/20091105 Namoroka/3.6b2pre ID:20091105033812

Comment 15

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

Comment on attachment 408535 [details] [diff] [review]
make reftest timeout messages a bit more useful

r=dbaron, although I probably should have suggested months ago that Jesse review this instead...

Comment 16

[Phil Ringnalda (:philor)]

I'm pretty sure you rolled the reftest harness changes of attachment 408535 [details] [diff] [review] into your rewrite, since the messages sound a bit familiar. I landed the re-enable the commented-out reftest part in http://hg.mozilla.org/mozilla-central/rev/c14987c4d6a0.