Closed
Bug 1092842
Opened 10 years ago
Closed 10 years ago
Windowed Flash is improperly clipped by content on youtube
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox33 wontfix, firefox34+ verified, firefox35+ verified, firefox36+ verified)
VERIFIED
FIXED
mozilla36
People
(Reporter: jimm, Assigned: roc)
References
()
Details
(Keywords: regression)
Attachments
(3 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
tnikkel
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR:
1) load the test case url
2) wait until video loads, then scroll up
result: the plugin window isn't properly clipped by the white content header.
Last good revision: 82df3654cd80 (2014-07-23)
First bad revision: a91ec42d6a9c (2014-07-24)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=82df3654cd80&tochange=a91ec42d6a9c
Flash Version: 15.0.0.189
Reporter | ||
Comment 1•10 years ago
|
||
bug 1041530 fixed a similar issue in glass, that fix landed on 2014-07-31. Looks like this is a regression from bug 1022612.
Also, the work in bug 1022612 landed on aurora 2014-07-24 and this regression is in that release as well.
Blocks: 1022612
tracking-firefox34:
--- → ?
Comment 2•10 years ago
|
||
I can reproduce on 34-36 on Windows 7. I cannot reproduce on OSX. As noted, this is an issue related to plug-ins and, as such, does not reproduce with every YouTube video.
Tracking as this is a regression, which I would like to fix. However, this issue is not severe enough to hold the 34 release.
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
Comment 3•10 years ago
|
||
Roc - You're listed as the owner of bug 1022612, which Jim expects caused this regression. Can you take this one?
Flags: needinfo?(roc)
Keywords: regression
Assignee | ||
Comment 4•10 years ago
|
||
I think this testcase covers what we're seeing with Youtube.
The nsDisplayTransform for the DIV is not marked as opaque, and so we don't subtract it (or the opaque regions of any of its contents) from the visible region for the plugin.
nsDisplayTransform only returns an opaque region if its entire contents are opaque, and that doesn't happen because the bottom-border part isn't known to be opaque. Actually the background color of the DIV nominally covers its entire border-box, but we clip it to the padding area because we know the opaque border is going to be painted outside it.
I have no idea how this was working before.
Assignee: nobody → roc
Flags: needinfo?(roc)
Assignee | ||
Comment 5•10 years ago
|
||
I've been trying to think of a reason why we need the display item cliprect to have the exclude-borders optimization, but I can't think of one.
Attachment #8517223 -
Flags: review?(tnikkel)
Updated•10 years ago
|
Attachment #8517223 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Reftests fail with this...
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a5fb4d7fc918
These tests fail on all platforms:
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/border-radius/corner-1.html | image comparison (==), max difference: 98, number of differing pixels: 56
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/border-radius/clipping-3.html | image comparison (==), max difference: 95, number of differing pixels: 70
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/456219-2.html | image comparison (==), max difference: 38, number of differing pixels: 156
Probably should just have more fuzz. They all already have fuzz and visually the results are fine.
Mac has a lot stuff like this:
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/backgrounds/vector/tall--cover--nonpercent-width-omitted-height-viewbox.html | image comparison (==), max difference: 1, number of differing pixels: 4
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/talos-slave/test/build/tests/reftest/tests/layout/reftests/backgrounds/vector/tall--cover--nonpercent-width-percent-height-viewbox.html | image comparison (==), max difference: 1, number of differing pixels: 4
Looks like a difference where we stroke a CSS border rectangle compared to the same thing drawn with SVG. Not sure what's up with that.
Windows scores an unexpected pass:
REFTEST TEST-UNEXPECTED-PASS | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/border-radius/curved-border-background-nogap.html | image comparison (==)
Need to think on this a bit.
Assignee | ||
Comment 7•10 years ago
|
||
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/456219-2.html | image comparison (==), max difference: 38, number of differing pixels: 156
This one doesn't have fuzz and moreover, the test shows a visual regression where stray pixels appear at the outside of rounded corners.
Assignee | ||
Comment 8•10 years ago
|
||
Different approach: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8001bce5a078
Assignee | ||
Comment 9•10 years ago
|
||
That gets a lot of weird off-by-1 errors on the corners of rectangles, on Mac only.
Assignee | ||
Comment 10•10 years ago
|
||
Bug 1097437 has a patch that should work around the Mac issues I saw.
Depends on: 1097437
Assignee | ||
Comment 11•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Assignee | ||
Comment 13•10 years ago
|
||
I just realized I forgot to get this re-reviewed before landing...
Attachment #8517223 -
Attachment is obsolete: true
Attachment #8521908 -
Flags: review?(tnikkel)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8521908 [details] [diff] [review]
fix v3
Approval Request Comment
[Feature/regressing bug #]: 1022612
[User impact if declined]: plugin incorrectly rendered above certain page content on Windows and Linux
[Describe test coverage new/current, TBPL]: there are some automated tests for this.
[Risks and why]: It regresses some sites including Youtube, seriously enough that they might try to work around it or users might complain. The patch just disables an optimization in a narrow set of cases.
[String/UUID change made/needed]: none
Note, if we take this then we also need to take the patch in bug 1097437 or tests will fail. Fortunately that patch is also low-risk.
Attachment #8521908 -
Flags: approval-mozilla-beta?
Attachment #8521908 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8521908 -
Flags: review?(tnikkel) → review+
Comment 15•10 years ago
|
||
Comment on attachment 8521908 [details] [diff] [review]
fix v3
Taking this in 34 beta9 as it is a visible regression on YouTube. I have asked QE to verify. Beta+ Aurora+
Attachment #8521908 -
Flags: approval-mozilla-beta?
Attachment #8521908 -
Flags: approval-mozilla-beta+
Attachment #8521908 -
Flags: approval-mozilla-aurora?
Attachment #8521908 -
Flags: approval-mozilla-aurora+
Comment 16•10 years ago
|
||
Comment 17•10 years ago
|
||
I just landed the **reftest manifest changes only** from bug 1096181 on aurora and beta, because those changes actually belonged in either bug 1097437 or this bug, which were just landed in both places:
https://hg.mozilla.org/releases/mozilla-aurora/rev/187e9fa68b67
https://hg.mozilla.org/releases/mozilla-beta/rev/c486cd17bebb
Updated•10 years ago
|
Flags: qe-verify+
Comment 18•10 years ago
|
||
I was able to reproduce the issue with the YouTube URL on Windows 7 x64, with Firefox 34 Beta 8 (Flash Version: 15.0.0.189). The issue no longer reproduces with:
- Firefox 34 Beta 9 - BuildID=20141113192814
- latest Firefox 36 Nightly - BuildID=20141113030201
Comment 19•10 years ago
|
||
Also fixed on Windows 7 x64 with latest Firefox 35 Aurora (BuildID=20141114004002).
Comment 20•10 years ago
|
||
This patch broke bug 1099445. can we get this backed out of 2.1 branch? ni? roc for help.
Flags: needinfo?(roc)
Assignee | ||
Comment 21•10 years ago
|
||
We could, as an emergency measure, but I think it's important to figure out bug 1099445 because it might be a deeper bug that just got exposed.
Flags: needinfo?(roc)
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•