Last Comment Bug 738392 - Plugin doesn't render inside a CSS transform on Mac
: Plugin doesn't render inside a CSS transform on Mac
Status: VERIFIED FIXED
[qa!]
: regression
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Edwin Flores [:eflores] [:edwin]
:
Mentors:
http://ksso.net/~alex/ff_bug/moz-tran...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 12:04 PDT by Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
Modified: 2012-06-07 07:14 PDT (History)
9 users (show)
dholbert: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
+
verified


Attachments
Fix (2.39 KB, patch)
2012-03-22 16:14 PDT, Edwin Flores [:eflores] [:edwin]
no flags Details | Diff | Splinter Review
Fix (2.38 KB, patch)
2012-03-22 16:33 PDT, Edwin Flores [:eflores] [:edwin]
roc: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Screenshot of behavior in FF10 (250.20 KB, image/png)
2012-05-02 13:06 PDT, scott
no flags Details

Description Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-22 12:04:52 PDT
See http://ksso.net/~alex/ff_bug/moz-transform.html.

I suspect this is because all Mac plugins are nominally "windowed" even though we can actually render them inside a transform.
Comment 1 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-22 12:12:40 PDT
Edwin, I can help you diagnose and fix this.
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-22 14:04:55 PDT
One thing we should look into is why the test checked in in bug 644832 did not catch this bug on Mac.
Comment 3 Edwin Flores [:eflores] [:edwin] 2012-03-22 16:14:49 PDT
Created attachment 608524 [details] [diff] [review]
Fix
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-22 16:20:26 PDT
Comment on attachment 608524 [details] [diff] [review]
Fix

Review of attachment 608524 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsPresContext.cpp
@@ +2486,5 @@
>        nsPtrHashKey<nsObjectFrame>* entry =
>          aClosure->mAffectedPlugins.GetEntry(f);
>        // Windowed plugins in transforms are always ignored, we don't
>        // create configurations for them
> +      if (entry && (!aInTransform || !f->PaintedByGecko())) {

This should be f->PaintedByGecko() --- no "not"

::: layout/generic/nsObjectFrame.cpp
@@ +2171,5 @@
> +#ifdef XP_MACOSX
> +  return false;
> +#else
> +  return !!mWidget;
> +#endif

Other way around. Plugins with widgets aren't painted by Gecko; plugins without widgets are.
Comment 5 Edwin Flores [:eflores] [:edwin] 2012-03-22 16:33:52 PDT
Created attachment 608534 [details] [diff] [review]
Fix
Comment 6 Daniel Holbert [:dholbert] 2012-03-23 11:23:30 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e18c3a6535a

It sounds like we could add a reftest for this, no?
Comment 7 Ed Morley [:emorley] 2012-03-24 13:58:07 PDT
https://hg.mozilla.org/mozilla-central/rev/3e18c3a6535a
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-25 18:59:21 PDT
(In reply to Daniel Holbert [:dholbert] from comment #6)
> It sounds like we could add a reftest for this, no?

There is one, and it was already passing, because our test plugin doesn't need the fix here. The paint events we send to the plugin work fine. The problem this patch fixes is that Flash seems to depend on the plugin's NSView having the right dimensions. I'm not sure how to test that since I can't even figure out how the Mac test plugin could discover its NSView; Flash must be using some horrible hack.
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-25 19:01:24 PDT
Comment on attachment 608534 [details] [diff] [review]
Fix

Review of attachment 608534 [details] [diff] [review]:
-----------------------------------------------------------------

Risk analysis: the patch clearly only affects Mac plugins in transforms, which are rare but completely broken (for Flash) without this patch.
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-25 19:05:58 PDT
Apparently this worked in FF10 (not sure why) which is why I'm nominating this as a regression.
Comment 11 Alexander Casassovici 2012-03-26 01:15:48 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> Apparently this worked in FF10 (not sure why) which is why I'm nominating
> this as a regression.

On FF10 and below the video did render but the controls were not clickable since the mouse X-Y position was not read properly by the flash player... (try right-click on the video, you will see the context menu open with a weird offset"
So we can't really say it was "working" in FF10 - the bug was just less blocking....
Comment 12 Alex Keybl [:akeybl] 2012-03-26 14:06:27 PDT
Comment on attachment 608534 [details] [diff] [review]
Fix

[Triage Comment]
Approving for Aurora 13 and Beta 12 given the limited risk that this change would introduce and the fact that the issue got worse in FF11.
Comment 13 Alex Keybl [:akeybl] 2012-03-26 14:08:43 PDT
Adding qawanted and regressionwindow-wanted since it doesn't seem as if we fully understand why the issue in https://bugzilla.mozilla.org/show_bug.cgi?id=644832#c13 has reappeared.
Comment 14 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-26 19:22:14 PDT
I've tried all the available test sites from bug 644832 and all of them render flash content correctly in the latest Nightly on Mac. If this regressed, I'm not seeing it.

http://uniboard-misc.s3.amazonaws.com/plugin_transform.html
http://scu.bz/p/1716
http://ksso.net/~alex/ff_bug/moz-transform.html
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-26 20:43:16 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b82d6245ac1
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-27 03:13:20 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/3b8bc668c24d
Comment 17 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 11:36:23 PDT
Bump. Please advise RE comment 14 and qawanted.
Comment 18 Alex Keybl [:akeybl] 2012-03-29 15:42:49 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #17)
> Bump. Please advise RE comment 14 and qawanted.

We expect this to be fixed in the latest nightlies now that it landed in Comment 7 - I wanted to get a regression window for when the behavior changed as noted in Comment 10 and Comment 11 (rendering worked in FF10, doesn't in FF11).
Comment 19 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-29 16:03:18 PDT
Considering I am unable to reproduce the issue (see comment 14) I don't see how I can get you a regression range. Can you advise something else I should try?
Comment 20 Alex Keybl [:akeybl] 2012-03-30 13:13:13 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #19)
> Considering I am unable to reproduce the issue (see comment 14) I don't see
> how I can get you a regression range. Can you advise something else I should
> try?

It sounds like roc was able to repro the issue on nightlies from around 2012-03-22. Hopefully he can help you with exact STR.
Comment 21 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-04-01 20:09:46 PDT
Actually Edwin reproduced it. However, I think we're in the realm of diminishing returns here; no need to keep working on independent verification, thanks.
Comment 22 Vlad [QA] 2012-04-02 06:45:49 PDT
Setting resolution to verified fixed on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0 beta 3

I have tried the example from the description and also the links from comment14 and the plug-in is rendering as expected.
Comment 23 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-02 09:35:03 PDT
Removing QAWANTED as per comment 21. Please re-add if there is something more QA can do.
Comment 24 scott 2012-05-02 13:06:52 PDT
Created attachment 620443 [details]
Screenshot of behavior in FF10

This is how overflow: hidden & border-radius behaved in FF10 with a Flash object.
Comment 25 scott 2012-05-02 13:09:57 PDT
This regression is not fully corrected. In FF10 authors could use `overflow: hidden;` with `border-radius` for elements containing Flash embeds and the corners of the Flash embed would be cropped. This is the most desirable behavior. Safari (Version 5.1.5 (7534.55.3)) does not crop the corners.

FF11 introduced an issue that is still existent in FF12 where the Flash object *disappears* when using overflow: hidden and border-radius. You can a simplified example of this here: http://www.edliohs.org/bug_testcases/ff_flash/index.jsp 

You should see rotating images, instead in FF11/12 you'll see a pink rounded rectangle as the Flash object disappears. Toggling border-radius or overflow: hidden in Firebug will cause the Flash to appear. I've attached a screenshot of this same test case in FF10. Please let me know if I can provide more info or testing. I would hope that Firefox will at least do what Safari does here and not cause the embedded object to disappear, even if the corners are not cropped.
Comment 26 scott 2012-05-02 13:11:52 PDT
And actually on closer reading this may be a separate issue though likely related. If so I apologize. The original test case provided above is not loading.
Comment 27 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-05-02 16:54:52 PDT
Separate issue --- please file bug :-)
Comment 28 Vlad [QA] 2012-05-16 08:32:26 PDT
I have tested this on
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20100101 Firefox/13.0 beta 3
and the plugin is rendering as expected.
I have tried the links from comment14, the link from the description is not working anymore.

The plugin is rendering also on Firefox 10 and 11. Is that expected?
Comment 29 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-16 09:50:39 PDT
(In reply to Vlad [QA] from comment #28)
> The plugin is rendering also on Firefox 10 and 11. Is that expected?

Yes. I believe the regression occurred around Firefox 12.
Comment 30 Vlad [QA] 2012-06-07 07:14:29 PDT
Verified this on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0 beta 6 (1st Firefox 14)

The plugin is rendering as expected. From comment14, only one link is working.

Setting resolution to Verified.

Note You need to log in before you can comment on or make changes to this bug.