475723 - -moz-box-shadow does not work properly on <embed>

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[eisa01]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_6; nb-no) AppleWebKit/525.27.1 (KHTML, like Gecko) Version/3.2.1 Safari/525.27.1
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090128 Minefield/3.2a1pre

For an empty <object> with defined height and width, no shadow is applied.
For an <object> with contents, the shadow is displayed wrong (too low).

Reproducible: Always




See test case
I also discovered that if I set the background to e.g. red, it is not displayed for the empty <object>, but for the <object> with contents it is filled, but you're not supposed to see it because the flash animation should block it.

Comment 1

[eisa01]

Attachment: Test case

Comment 2

[eisa01]

Attachment: Expected result

Left is Mozilla, right is Safari. I expect the result to be similar.

Comment 3

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

Confirmed.

Comment 4

[Michael Ventnor]

This is not a box-shadow bug. This is because of a difference between webkit and gecko. Webkit seems to treat objects as block frames, so it is able to accept an explicit size and use that. Gecko treats objects as inline frames so it can't accept explicit sizes, you'll see that backgrounds and borders give the same symptoms.

Setting the object frames to display: block fixes the issue.

Comment 5

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

(In reply to comment #0)
> For an empty <object> with defined height and width, no shadow is applied.

That's because an empty <object> simply triggers fallback to the <object>'s inline content. The <object> itself is treated like a span --- an inline element --- which does not support width/height sizing. Our rendering is correct in this case, or at least, it's not violating any spec.

> For an <object> with contents, the shadow is displayed wrong (too low).

That's because we're rendering the <embed>, which has no shadow. The object element again is treated just as an inline element, and again the width and height are ignored.

Now, if you put the box-shadow rule on the embed, then it should work. But it doesn't --- and that's a real bug.

Comment 6

[Michael Ventnor]

Attachment: Patch

Turns out we needed to include overflow in the frame's bounds so the box shadow isn't "optimized" away.

Comment 7

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

We should have a reftest here, but I guess it depends on the test plugin working on all platforms.

Comment 8

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

http://hg.mozilla.org/mozilla-central/rev/2676947cb41c

Comment 9

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

Comment on attachment 359453 [details] [diff] [review]
Patch

Straightforward fix for a bug in a new feature

Comment 10

[(not currently active) Ted Mielczarek]

(In reply to comment #7)
> We should have a reftest here, but I guess it depends on the test plugin
> working on all platforms.

You can make it platform-specific at the moment:
http://mxr.mozilla.org/mozilla-central/source/modules/plugin/test/reftest/reftest.list

We'll get the windows/gtk drawing code landed soon, anyway. Might as well write the test!

Comment 11

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

Sorry, but that doesn't work on trunk with the checked-in patch. I did a test with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre and the behavior is still the same. Identically results I get on Windows.

Comment 12

[eisa01]

Comment on attachment 359283 [details]
Test case

Test case turned out to be incorrect

Comment 13

[eisa01]

Attachment: Test case #2

Now applies the style to the <embed> element

Comment 14

[eisa01]

(In reply to comment #11)
> Sorry, but that doesn't work on trunk with the checked-in patch. I did a test
> with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
> Gecko/20090201 Minefield/3.2a1pre and the behavior is still the same.
> Identically results I get on Windows.

It works just fine for me on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090201 Minefield/3.2a1pre

The test case was incorrect as it applied the style to the <object> element

btw, I also want to say thanks for fixing this and bug 475197 so fast, I'm impressed :)

Comment 15

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

Thanks for the hint. I checked again with the above build and everything is fine. Marking as verified.

Comment 16

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e98e69040f91

Comment 17

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

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090204020327

For now no verification on Windows is possible because the gfx drawing code hasn't been landed yet. See comment 10. Ted, do you have a bug number we can add to the dependency list?

Comment 18

[(not currently active) Ted Mielczarek]

The Windows test plugin drawing code landed in bug 469830. The gtk2 plugin drawing code is bug 469831.

Comment 19

[Boris Zbarsky [:bzbarsky]]

See bug 478267.

Comment 20

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

I think we should back this out (and bug 478267) on branch to fix bug 479637 on branch. The work required to fix bug 479637 properly in a branch-safe way isn't justified by this fix.

On trunk, my work on plugin hoisting will fix bug 479637.

Comment 21

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

Backed out of 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/91b65591babb

So this is no longer fixed in 1.9.1, and we don't plan to fix it there.

Comment 22

[cmtalbert]

(In reply to comment #18)
> The Windows test plugin drawing code landed in bug 469830. The gtk2 plugin
> drawing code is bug 469831.

So, Aaron (cc'd) is looking at writing a test for this bug. However, if you use the test plugin, the box shadow is drawn in both Minefield and Shiretoko.  If you use the original flash based testcase, the box shadow is only drawn on Minefield.  We need some direction here about what's really going on in order to test this.  

I don't think we can countenance checking in Flash into reftests :)

Comment 23

[Michael Ventnor]

(In reply to comment #22)
> So, Aaron (cc'd) is looking at writing a test for this bug. However, if you use
> the test plugin, the box shadow is drawn in both Minefield and Shiretoko.  If
> you use the original flash based testcase, the box shadow is only drawn on
> Minefield.  We need some direction here about what's really going on in order
> to test this.  
> 
> I don't think we can countenance checking in Flash into reftests :)

I presume because the test plugin is windowless by default. Windowless plugins work fine, but plugins in windowed mode show this bug.

Comment 24

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

The test plugin supports wmode="window".

Comment 25

[Michael Ventnor]

And Linux doesn't support getting the contents of Windowed plugins in a consistent manner due to an unfortunate optimization of X.

Comment 26

[cmtalbert]

Attachment: A simple test that uses the test plugin

(In reply to comment #24)
> The test plugin supports wmode="window".
I've attached a simple test that uses this attribute on the embed tag and still displays the drop shadow in both minefield and shiretoko.

Minefield: (an old debug build) Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090423 Minefield/3.6a1pre

Shiretoko: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090508 Shiretoko/3.5b5pre

Comment 27

[Aaron Train [:aaronmt]]

Michael,

With the above test case running on 

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b5pre) Gecko/20090511 Shiretoko/3.5b5pre

I get no visual whatsoever (blank white screen).

Is this a result in reference to your comments above in regards to Linux?

Comment 28

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

(In reply to comment #26)
> > The test plugin supports wmode="window".
> I've attached a simple test that uses this attribute on the embed tag and still
> displays the drop shadow in both minefield and shiretoko.
> 
> Minefield: (an old debug build) Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5;
> en-US; rv:1.9.2a1pre) Gecko/20090423 Minefield/3.6a1pre
> 
> Shiretoko: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
> rv:1.9.1b5pre) Gecko/20090508 Shiretoko/3.5b5pre

Mac never uses the windowed plugin path. Only Windows and Linux do.

Comment 29

[(not currently active) Ted Mielczarek]

(In reply to comment #27)
> Michael,
> 
> With the above test case running on 
> 
> Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b5pre) Gecko/20090511
> Shiretoko/3.5b5pre
> 
> I get no visual whatsoever (blank white screen).
> 
> Is this a result in reference to your comments above in regards to Linux?

Aaron: you'd need a self-compiled build of Firefox in order to have the test plugin available.

roc: I see a drop-shadow in Clint's test with a fairly recent Windows trunk build.

Comment 30

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

Yeah, because this is fixed on trunk. On 1.9.1 you should see the bug.

Comment 31

[Michael Ventnor]

(In reply to comment #27)
> Michael,
> 
> With the above test case running on 
> 
> Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b5pre) Gecko/20090511
> Shiretoko/3.5b5pre
> 
> I get no visual whatsoever (blank white screen).
> 
> Is this a result in reference to your comments above in regards to Linux?

No, my comment was about if you were going to make a reftest about this; on Linux we don't support compositing of windowed plugins onto <canvas> and the like. Your issue is due to this bug. It doesn't happen on Mac because Mac doesn't support windowed plugins.