Closed
Bug 770945
Opened 12 years ago
Closed 12 years ago
"Simple events" on HTML5 media shouldn't be cancelable
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jeremy, Assigned: kinetik)
References
Details
Attachments
(2 files, 1 obsolete file)
4.42 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
3.18 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_3) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.47 Safari/536.11 Steps to reproduce: I used addEventListener to add listeners for the "play", "playing", "pause", "seeking", "seeked" and "timeupdate" events on a <video> element, twice. The first time I just logged the event objects to the console. The second time I called their .preventDefault() method first. Actual results: The .cancelable property was set true on all of the events. Calling .preventDefault() did not cancel any of the actions, though it did set the .defaultPrevented property to true. Expected results: In the current working draft of the specification for HTML5 media elements and events, all of the above are described as "simple events". Simple events are not supposed to be cancelable unless otherwise specified, and it was not otherwise specified for these events. Therefore I would expect the .cancelable property to be false and .preventDefault() to have no effect, even on the value of .defaultPrevented. Tested in Firefox Nightly. (Chrome, Safari and Opera's stable releases exhibit the same behaviour. Perhaps I'm confused, but the specification seem unambiguous.)
Updated•12 years ago
|
Component: Untriaged → General
Product: Firefox → Core
Updated•12 years ago
|
Component: General → Video/Audio
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → kinetik
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 1•12 years ago
|
||
Oops, we fixed bubbling in bug 608630 but missed cancellation. Simple patch attached. Mochitests are green locally; pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=6a2f068a9fb4 This patch also changes the MozAudioAvailable events to not cancel *or* bubble, which seems like what we'd want.
Attachment #651620 -
Flags: review?(roc)
If Chrome, Safari, Opera and us (what about IE?) all have already made these events cancelable, shouldn't we change the spec instead of changing our behavior?
Assignee | ||
Comment 3•12 years ago
|
||
There's no behaviour specified for cancelling media events, and I don't know what'd mean to be able to cancel most of them. From a quick look, I can't see any code in WebKit/Chromium's media implementation that handles media events being cancelled.
OK. I guess it probably doesn't make any difference to Web content. You uploaded the wrong patch though :-)
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 651620 [details] [diff] [review] patch v0 (wrong patch) Oops.
Attachment #651620 -
Attachment description: patch v0 → patch v0 (wrong patch)
Attachment #651620 -
Attachment is obsolete: true
Attachment #651620 -
Flags: review?(roc)
Assignee | ||
Comment 6•12 years ago
|
||
Correct patch attached this time. And a little more history: The spec used to say that simple events were not bubbling but were cancellable and was changed to the current wording in in April 2009: http://html5.org/tools/web-apps-tracker?from=2991&to=2992 Our implementation of nsHTMLMediaElement::DispatchEvent was committed July 2008 and initially creating bubbling and cancellable events--bubbling was fixed in November 2010. WebKit's implementation dates from late 2007 and created cancellable non-bubbling events from inception. So I think this is just a case of the spec changing and nobody noticing. I've made a note to file bugs against the other browsers to fix their implementations.
Attachment #651646 -
Flags: review?(roc)
Attachment #651646 -
Flags: review?(roc) → review+
Assignee | ||
Comment 7•12 years ago
|
||
WebKit bug (also covers Chrom{e,ium}): https://bugs.webkit.org/show_bug.cgi?id=94065 Opera bug: DSK-372112@bugs.opera.com
Assignee | ||
Comment 8•12 years ago
|
||
IE 9's events aren't cancellable and don't bubble. Nor are IE 10 (pp2, I don't have Windows 8 to test later versions).
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/806f04d54ab5
Flags: in-testsuite?
Target Milestone: --- → mozilla17
Assignee | ||
Comment 10•12 years ago
|
||
I realized that I should not have changed InitAudioAvailableEvent (since DOM events inits share common parameters at the start, and my change breaks that pattern), and instead should just pass the appropriate args to InitAudioAvailableEvent from nsHTMLMediaElement. No need for a review, since this is just a partial revert of my previous patch. https://hg.mozilla.org/integration/mozilla-inbound/rev/b83bfbfa9b60
Assignee | ||
Updated•12 years ago
|
Attachment #652677 -
Attachment description: patch v0 partial revent → patch v0 partial revert
Comment 11•12 years ago
|
||
(In reply to Matthew Gregan [:kinetik] from comment #10) > Created attachment 652677 [details] [diff] [review] > patch v0 partial revert > > I realized that I should not have changed InitAudioAvailableEvent (since DOM > events inits share common parameters at the start, and my change breaks that > pattern), and instead should just pass the appropriate args to > InitAudioAvailableEvent from nsHTMLMediaElement. > > No need for a review, since this is just a partial revert of my previous > patch. > > https://hg.mozilla.org/integration/mozilla-inbound/rev/b83bfbfa9b60 Indeed. I don't understand how this passed review in the first place.
Assignee | ||
Comment 12•12 years ago
|
||
This was merged to m-c but the bug was never updated: http://hg.mozilla.org/mozilla-central/rev/806f04d54ab5 http://hg.mozilla.org/mozilla-central/rev/b83bfbfa9b60
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•