638118 - Change mozilla.com videos to use html5 video preload attribute

Status: RESOLVED FIXED

(www.mozilla.org :: General, defect)

Description

[Wesley Johnston (:wesj)]

I recently made a change to Fennec so that videos with no preload attribute don't preload anything in them (bug 631058, still more work to be done there, but this part has landed). However, the video content at

http://www.mozilla.com/en-US/firefox/video/

relies on us preloading some amount of video (it checks if video.readyState will return HAVE_CURRENT before it will call video.play I believe). The change in Fennec is a bit controversial, but a low risk (easy?) fix would be to explicitly include preload=metadata on the video elements. Or modify mozilla-video-tool.js to not rely on these attributes.

Comment 1

[(no longer active)]

Hmm, the suggestion in comment 0 would leave the rest of the sites on the web which do this affected, right?

Comment 2

[Wesley Johnston (:wesj)]

Yes. Sites that assume we preload video without including a preload attribute may experience problems.

Comment 3

[(no longer active)]

(In reply to comment #2)
> Yes. Sites that assume we preload video without including a preload attribute
> may experience problems.

So, is that considered a bug on the web page or a bug in the browser?  If the former, why are we trying to modify what Fennec does here?

Comment 4

[Wesley Johnston (:wesj)]

Maybe I probably shouldn't have marked this blocking. This bug is filed against www.mozilla.com, not Fennec.

Comment 5

[Michael Morgan [:morgamic]]

Hey James - can you or Anthony pick this one up for the next cycle?  It's important for Fennec launch.

Comment 6

[James Long (:jlongster)]

Sure, I'll take it on.

Comment 7

[James Long (:jlongster)]

done in r85249

I just added the preload="metadata" attribute. That page actually doesn't use the mozilla-video-tools library to render the video. Is it good enough just to fix that one video?

Comment 8

[Wesley Johnston (:wesj)]

Hmm... I'm a bit confused. Looks to me like all of the pages use mozilla-video-tools to setup the overlay on top of the video, and to check if it has downloaded data.

Videos with no preload attribute won't preload anything in Fennec currently, so none of them will be playable without either

1.) changing mozilla-video-tools to not check if video has been preloaded or
2.) adding a preload attribute to the video.

Comment 9

[James Long (:jlongster)]

(In reply to comment #8)
> Hmm... I'm a bit confused. Looks to me like all of the pages use
> mozilla-video-tools to setup the overlay on top of the video, and to check if
> it has downloaded data.

You're right, now that I look closer. I haven't seen a video setup like this, I was looking for the VideoPlayer object, but this isn't a different feature of mozilla-video-tools.

If the library assuming that the video will be preloaded, it seems like the attribute should be added. I'll get the opinion of the author of mozilla-video-tools.

Comment 10

[James Long (:jlongster)]

I'll go ahead and make the library add the preload attribute. Is it ok if the preload attribute is added via javascript after the page loads?

It looks like Mike from silverorange wrote the library, so I'll try to get him to do a code review when I'm done.

Comment 11

[James Long (:jlongster)]

Attachment: patch to mozilla-video-tools for preload

Here's a patch that will do it. I'd like to get silverorange's opinion on this since they wrote the library.

Comment 12

[Michael Gauthier [:gauthierm]]

How about only setting the preload attribute in JS if it's not already set on the video element or if it is set to 'none'?

Setting the preload in JS rather than HTML causes about 200 ms of delay loading the video. Only setting it if it's not set or set to 'none' would still fix the problem you are experiencing with Fennec, and still allow setting in HTML on the video page.

Comment 13

[James Long (:jlongster)]

Good idea, I'll change it.

200ms shouldn't matter since the user won't even press "play" by then, right?

Comment 14

[James Long (:jlongster)]

Attachment: new patch for mozilla-video-tools

Comment 15

[Michael Gauthier [:gauthierm]]

Patch looks good. We should keep the existing preload="metadata" on the video page though.

200ms is not a lot but makes the video thumbnail take longer to appear and thus diminishes the feeling of responsiveness.

Comment 16

[James Long (:jlongster)]

cool. done in r85303 (preload attribute is added to the specific page in r85249)

do you have a requirement for when this goes out?

Comment 17

[raymond [:retornam] (inactive)]

qa-verified-trunk  http://www-trunk.stage.mozilla.com