464371 - <video> fires multiple load events, controls should be <button>s

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Justin Dolske [:Dolske]]

<video src="foo.ogg" onload="alert(event.originalTarget.localName)"></video>

This gives 3 alerts (video, image, image)... One for each of the two images in the binding, and one for the video itself. This problematic, because content certainly isn't going to be expecting the load events from the control images.

Ideally XBL would suppress these events automagically, but if markup/code in the binding could suppress them that would be an ok workaround for me.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Would it be possible to use CSS background images in the binding?
Any code in XBL can't suppress capture phase of the events.
And because load events are special, they don't propagate to chrome,
so chrome can't suppress them either.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Or if really needed we could add some attribute to XBL <content> which tells
the event names which should not propagate out of the anonymous content.
(Probably a bit tricky to implement, but doable.)

So I think if CSS background images just are possible here, using them
would be good enough work-around.

Comment 3

[Justin Dolske [:Dolske]]

Hmm, yeah, I'll give background images a shot.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, XBL by default can't suppress the events, but we could add something like comment 2 suggests.  It might not be that tricky if we handle it at the event retargeting step.

Comment 5

[Justin Dolske [:Dolske]]

Enn says these controls should be <button>s anyway, so I'll look at fixing both at the same time.

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Tried modifying test_videocontrols.html to test this, by changing test 1 to be invoked by a |load| event from the video element. Unfortunately bug 464377 is still causing the video itself to fire multiple load events, which causes the modified test to fail.

But I did confirm that event.originalTarget.localName was "VIDEO" on those extra events, so the button images should be ok.

Comment 7

[Justin Dolske [:Dolske]]

(Also, note that Bugzilla's diff viewer is broken, and doesn't show the 4 .png images added to pinstripe's jar.mn. They're in the raw patch.)

Comment 8

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/69d77c5270ed

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 354018 [details] [diff] [review]
Patch v.1

a191=beltzner

Comment 10

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/897ccdad1f2f

Comment 11

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre)
Gecko/20090413 Shiretoko/3.5b4pre