486673 - white space around video in object element causes unexpected scrollbars

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Wicher Minnaard]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3) Gecko/20090329 Gentoo Firefox/3.1b3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3) Gecko/20090329 Gentoo Firefox/3.1b3

Say I'd have an embedded video for display, something like this:

<object data="http://example.com/video.ogv"  type="application/ogg" autostart="false" width="400" height="300"/>

Works fine in Firefox < 3.1. Firefox >= 3.1 will not launch mplayerplug-in (or whatever), but will play this using the builtin Theora player. That's a good thing.
However, it will show the video in a viewport that's too small: only part of the video is shown and there are scrollbars to the bottom and the right of the video display area. 

Workaround is to do something like this, for a 400x300 video:
<object data="http://example.com/video.ogv"  type="application/ogg" autostart="false" width="421" height="316"/>
This gets rid of the scrollbars, but it's not a pretty solution since plugins in Firefox < 3.1 will render this using a plugin and scale it up.


Reproducible: Always

Steps to Reproduce:
1. Embed 400x300 video into an HTML page with <object data=.../>
2. View page.

Actual Results:  
Video is not fully displayed; there are scrollbars in the video display area.

Expected Results:  
Video area should be 400x300, as specified in the <object>-element. This would give consistent behaviour across Firefox < 3.1 and Firefox >= 3.1. Right now the embedded Theora player will 'break' pages that used to work fine.

Comment 1

[Daniel.S]

I can reproduce this. When I display a video by using an object element, there is some white space rendered around the video that is causing the scrollbars.

Do we load ourselves as plugin to display the video?

@reporter: 
A workaround is to put the object element inside the video element. Firefox 3 doesn't know the video element, so it's completely ignored. Instead, the object inside would be shown. Like..

<video>
 <object></object>
</video>

Comment 2

[Daniel.S]

Attachment: testcase

Comment 3

[Wicher Minnaard]

@Daniel:
Thanks, but the construct
<video>
 <object></object>
</video>
works if you want to show video in 3.1 and prevent it from showing up in < 3.1. I could use this to put the 3.1-specific workaround in, but I'd still need to put a < 3.1 - style <object> somewhere, with sane width and height attributes. Which would then be played by 3.1 as well, so you'd have *two* video's being played in your 3.1, one of which would have the goofy whitespace stuff. < 3.1 would display one video, correctly.

As far as I can tell there is no workaround except the scaling trick I mentioned, or you'd have to fiddle around with javascript. Or am I missing something?

Anyway, we cannot expect all of the WWW to somehow apply these workarounds and de-apply them when this gets fixed, so I guess this blocks a 3.5-release unless it's OK to ruin the user experience of pages with embedded theora that used to work perfectly in 3.0+some plugin ...
I don't think that's OK, especially since HTML5<video> is one of the most heavily anticipated features in 3.5. More than a few early adopting webmasters, including me, will expect this to mark the beginning of the end of the plugin-and-codec-and-FLV-hell that is embedded video. Would be a bit of a disappointment if it causes more issues than it solves, and such would greatly hamper Theora uptake.

An alternative to fixing this would be to disable the builtin theora player for <object>-elements, returning to < 3.1-style plugin behaviour. This is what I would expect: if I want Firefox to play my Theora, I'll use the fancy new HTML5 <video> tag. If I keep using the <object>-element, I'll keep getting the legacy behaviour (plugin).
On the other hand, the current approach (if done correctly) of "hijacking" the legacy <object>-behaviour will greatly speed up Theora uptake since, no matter which embedding element you choose or on which platform your users are, you could just tell them "get Firefox 3.5 for video playback" and be done with it.

Comment 4

[cajbir (:cajbir)]

(In reply to comment #3)
> I could use this to put the 3.1-specific workaround in, but I'd still need to
> put a < 3.1 - style <object> somewhere, with sane width and height attributes.
> Which would then be played by 3.1 as well, so you'd have *two* video's being
> played in your 3.1, one of which would have the goofy whitespace stuff. < 3.1
> would display one video, correctly.

This should work:

<video src="http://example.com/video.ogv">
<object data="http://example.com/video.ogv"  type="application/ogg"
autostart="false" width="400" height="300"/>
</video>

On a 3.1 or greater browser it will use the video element. On a 3.0 or less browser it will use the object element.  The child 'object' element will not be instantiated by the 3.1 or greater browsers. I've used this for fallback myself.

I do see your point regarding the existing usage of <object> content being broken by the internal player and I do think that should be fixed.

Comment 5

[Wicher Minnaard]

Attachment: Testcase for simultaneous <video> and <object> theora video playing

Comment 6

[Wicher Minnaard]

(In reply to comment #4)
> This should work:
> 
> <video src="http://example.com/video.ogv">
> <object data="http://example.com/video.ogv"  type="application/ogg"
> autostart="false" width="400" height="300"/>
> </video>
> 
> On a 3.1 or greater browser it will use the video element. On a 3.0 or less
> browser it will use the object element.  The child 'object' element will not be
> instantiated by the 3.1 or greater browsers. I've used this for fallback
> myself.
That's weird. On my Firefox (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3) Gecko/20090329 Gentoo Firefox/3.1b3) *both* the elements get played, but only the video-element is visible. I've attached a testcase for this scenario.
When I open the testcase, the following happens:
1. Top video in the <video> element is shown. It's paused at frame 0.
2. Meanwhile, the video in the <object> element is playing. I can't see it, but I hear the sound playing.
3. When I right-click the top video (from the <video> element) I can choose "play" and the video will start playing. Sound from the two videos is mixed together.

Don't you get this behaviour when opening the attached testcase?

Comment 7

[Wicher Minnaard]

Attachment: Testcase for simultaneous <video> and <object> theora video playing

This time entity-encoded ;-)

Comment 8

[cajbir (:cajbir)]

That definitely is a bad bug (referring to comment 6). Can you spin off into a new one?

Comment 9

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

Mmm, seems like we should apply some special style for non-toplevel video documents that scales the video to the object frame size. The question is whether it should preserve aspect ratio or not. I suppose it should.

Comment 10

[Wicher Minnaard]

Spun the <object> within <video> issue off as bug 487398 .

Comment 11

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

Attachment: fix

This patch ensures that an <object> or <iframe> referrring to a video resource displays the video sized to fill the object, preserving aspect ratio.

Images are handled differently, though ... <object> referring to an image actually creates an nsImageFrame instead of a nsImageDocument subdocument. Is it worth trying to do the same kind of thing for video?

Comment 12

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

The advantage of doing that would be that <object> could get intrinsic size data from the video.

The disadvantage is that there is quite a bit of image-loading-related code in nsObjectLoadingContent and I'm not sure if trying to add video in the same way would make sense...

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Is it worth trying to do the same kind of thing for video?

I think so, yes.  You'd need the following pieces:

1)  A new object type TYPE_VIDEO (and rearrange the types so that TYPE_NULL == 1
    and we never have to change it again).
2)  Inheriting nsObjectLoadingContent from whatever will provide the correct
    intrinsic state for video (presumably a common base-class for
    nsObjectLoadingContent and nsHTMLVideoElement).
3)  Passing the existing video channel to whoever needs to get it (this already
    works in the video document, right?).
4)  A small change to frame construction to construct a video frame here.

There really isn't that much image code in nsObjectLoadingContent, from what I can see.  A lot of the nsIImageLoadingContent casts are just to get an canonical nsISupports*.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 371808 [details] [diff] [review]
fix

This looks fine; we want it regardless of the other change, because someone might do <iframe> pointing to a video.

Comment 15

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

We should probably do something like this for non-toplevel nsImageDocuments too, then? I know there was a bug about our image zoom UI causing confusion when it's activated in an IFRAME...

Comment 16

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

Pushed http://hg.mozilla.org/mozilla-central/rev/fe5ce245a492

Comment 17

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

I marked the tests random:
http://hg.mozilla.org/mozilla-central/rev/17740538687d
One test failed on Linux:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239442982.1239451114.16946.gz&fulltext=1
It looks like we fired the document load event before the <object> is able to display the video...

Comment 18

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

Filed bug 486673 on comment #13.

Comment 19

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4f5825bd3261

Comment 20

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/58a58d8959a5

Comment 21

[Aakash Desai [:aakashd]]

verified FIXED on builds using the attached test case:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre (.NET CLR 3.5.30729) ID:20090604045218

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090604042555