738392 - Plugin doesn't render inside a CSS transform on Mac

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

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

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) (email my personal email if necessary)]

Edwin, I can help you diagnose and fix this.

Comment 2

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

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 [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Fix

Comment 4

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

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 [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Fix

Comment 6

[Daniel Holbert [:dholbert]]

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]]

https://hg.mozilla.org/mozilla-central/rev/3e18c3a6535a

Comment 8

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

(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) (email my personal email if necessary)]

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) (email my personal email if necessary)]

Apparently this worked in FF10 (not sure why) which is why I'm nominating this as a regression.

Comment 11

[Alexander Casassovici]

(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]]

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]]

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

[u279076]

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) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8b82d6245ac1

Comment 16

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

https://hg.mozilla.org/releases/mozilla-beta/rev/3b8bc668c24d

Comment 17

[u279076]

Bump. Please advise RE comment 14 and qawanted.

Comment 18

[Alex Keybl [:akeybl]]

(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

[u279076]

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]]

(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) (email my personal email if necessary)]

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]]

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

[u279076]

Removing QAWANTED as per comment 21. Please re-add if there is something more QA can do.

Comment 24

[scott]

Attachment: Screenshot of behavior in FF10

This is how overflow: hidden & border-radius behaved in FF10 with a Flash object.

Comment 25

[scott]

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]

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) (email my personal email if necessary)]

Separate issue --- please file bug :-)

Comment 28

[Vlad [QA]]

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

[u279076]

(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]]

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.