Skip to content

Commit 58b8163

Browse files
committed
Closes #12 - I got hung up on being told this wasn't The Right Way(tm), but I don't believe there's *any* right way to sort out this race condition crap. Anyway, this change now means there shouldn't be any race condition issues - the downside being that the content isn't indexed (if video ever was) and doesn't work for those with JavaScript off :(
1 parent 3ba2931 commit 58b8163

File tree

1 file changed

+47
-11
lines changed

1 file changed

+47
-11
lines changed

demos/video.html

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
<title>Video</title>
22
<article>
3-
<video>
4-
<source src="assets/dizzy.mp4" />
5-
<source src="assets/dizzy.webm" />
6-
<source src="assets/dizzy.ogv" />
7-
</video>
3+
<p id="altmsg">It doesn't appear that your browser supports native video, sorry :(</p>
84
<p id="controls">
95
<input type="button" id="play" value="play">
106
<span id="position">00:00</span> / <span id="duration">loading...</span>
@@ -13,14 +9,49 @@
139
<p>Note that (at time of writing) <a href="http://webkit.org/" title="The WebKit Open Source Project">Webkit nightly</a> supports full screen mode, which will add a button above.</p>
1410
</article>
1511
<script>
16-
var video = document.querySelector('video'),
17-
togglePlay = document.querySelector('#play'),
18-
position = document.querySelector('#position'),
19-
using = document.querySelector('#using'),
12+
/*
13+
Note that this example used to contain the video inline in the HTML above.
14+
However, since it's been understood that there's the possible risk of a race
15+
condition (which I'll explain in a moment), it means the best way to prevent
16+
this problem is to generate the video element attaching the events, and only
17+
once all this is in place, assign the source and insert the element.
18+
19+
Other possible alternatives would be to listen on the window object for the
20+
loadedmetadata event and to check the event.target with the element we're
21+
interested in, but it would require the script at the top of the code -
22+
which would block, and that's something I prefer not to do.
23+
24+
Another alternative is to put inline event handlers in the markup, again
25+
this is something I prefer not to do.
26+
27+
I was called out by a couple of eager folk, one suggesting that what we had
28+
before wasn't The Right Way(tm) - which frankly, ultimately, since we can't
29+
attach the event handlers like we would anything else (though technically
30+
an image element is subject to the same problems), I argue that there's
31+
therefore No Right Way(tm) to achieve this. Anyway, this change is for
32+
them. I know they'll love me a little more for it. Maybe. Perhaps. ...
33+
34+
*/
35+
36+
37+
var container = document.getElementById('altmsg'),
38+
video = document.createElement('video'),
39+
source, // support will be detected
40+
togglePlay = document.getElementById('play'),
41+
position = document.getElementById('position'),
42+
using = document.getElementById('using'),
2043
ready = false,
21-
controls = document.querySelector('#controls'),
44+
controls = document.getElementById('controls'),
2245
fullscreen = null;
2346

47+
if (video.canPlayType('video/webm')) {
48+
source = 'assets/dizzy.webm';
49+
} else if (video.canPlayType('video/mp4')) {
50+
source = 'assets/dizzy.mp4';
51+
} else if (video.canPlayType('video/ogv')) {
52+
source = 'assets/dizzy.ogv';
53+
}
54+
2455
addEvent(togglePlay, 'click', function () {
2556
if (ready) {
2657
video.playbackRate = 0.5;
@@ -59,9 +90,14 @@
5990
video.webkitEnterFullScreen();
6091
});
6192
}
62-
6393
});
6494

95+
if (source) {
96+
video.src = source;
97+
console.log(video);
98+
container.parentNode.replaceChild(video, container);
99+
}
100+
65101
function asTime(t) {
66102
t = Math.round(t);
67103
var s = t % 60;

0 commit comments

Comments
 (0)