Media element should fire error event when src is a 404

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Audio/Video
P2
normal
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
x86
All
fixed1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 360390 [details]
Mochitest

If you have <video src="foo.ogg"></video> where "foo.ogg" resolves to a 404 error, the video element should send an error event to the video element after it fails to load.

See http://www.whatwg.org/specs/web-apps/current-work/multipage/video.html#dom-media-load "If the media data can be fetched but is in an unsupported format, or can otherwise not be rendered at all"
(Assignee)

Comment 1

9 years ago
Created attachment 360631 [details] [diff] [review]
Patch v1

Chnages nsHTMLMediaElement::Load() so that it matches the spec and dispatches errors asynchronously when it can't find a valid resource to load. Load() also sends network errors when appropriate. Made nsHTMLMediaElement::NetworkError() send async events to match spec. Updated mochitests so that they stop the error event propagating - now when you have a media element with no src/source, it posts an error event when it's added to a document. This error can bubble up to the mochitest's body's onerror handler, and cause it to report a test failure.

This patch is based on top of Roc's patch for bug 455654.
Assignee: nobody → chris
Attachment #360390 - Attachment is obsolete: true
Attachment #360631 - Flags: superreview?(roc)
Attachment #360631 - Flags: review?(chris.double)
Attachment #360631 - Flags: superreview?(roc) → superreview+

Updated

9 years ago
Attachment #360631 - Flags: review?(chris.double) → review+

Updated

9 years ago
Blocks: 471220
(Assignee)

Comment 2

9 years ago
Requesting blocking 1.9.1, this is needed for bug 464455 which is blocking 1.9.1.
Depends on: 455654
Flags: blocking1.9.1?
(Assignee)

Comment 3

9 years ago
(In reply to comment #2)
> Requesting blocking 1.9.1, this is needed for bug 464455 which is blocking
> 1.9.1.

I mean blocking bug 462455.
Keywords: checkin-needed
Whiteboard: [needs landing]
(Assignee)

Comment 4

9 years ago
Created attachment 361199 [details] [diff] [review]
v1.1 Unbitrotten

v1 unbitrotten. r+ doublec sr+ roc.
Attachment #360631 - Attachment is obsolete: true
Attachment #361199 - Flags: superreview+
Attachment #361199 - Flags: review+
If this is ready to land, mark it checkin-needed / [needs landing]?
Er, I guess it already is so marked. But on IRC you mentioned there was a problem with Wave. Is that resolved?
(Assignee)

Comment 7

9 years ago
There's a livelock which shows up in bug 462455, this is ready to go. Bug 462455's patch requires this one.

Comment 8

9 years ago
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/364777bc90b5
Keywords: checkin-needed

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2

Comment 9

9 years ago
Backed out due to test failures:
http://hg.mozilla.org/mozilla-central/rev/f51e5b101ca5
http://hg.mozilla.org/mozilla-central/rev/0088c5d48c8c

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234142462.1234145460.9210.gz
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/aspect-ratio-1b.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/aspect-ratio-2a.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/aspect-ratio-2b.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/basic-1.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/canvas-1a.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/canvas-1b.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/empty-1a.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/empty-1b.html | timed out waiting for reftest-wait to be removed (after onload fired)
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-unittest/build/layout/reftests/ogg-video/zoomed-1.html | timed out waiting for reftest-wait to be removed (after onload fired)
Status: RESOLVED → REOPENED
Flags: blocking1.9.1+ → blocking1.9.1?
Priority: P2 → --
Resolution: FIXED → ---
(Assignee)

Comment 10

9 years ago
Linux: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234142462.1234142465.3828.gz
OSX: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234142462.1234145460.9210.gz
Windows: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234142462.1234147619.15129.gz
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
(Assignee)

Comment 11

9 years ago
Created attachment 361458 [details] [diff] [review]
Patch v2

As v1.1, but has fix for broken unit tests. A case in nsHTMLMediaElement::nsMediaLoadListener::OnStartRequest() would trigger for local files, where InitializeDecoderForChannel() would succeed, but return no listener, and the previous patch would trigger a network error in that case.

Also changes access controls mochitest so that it always recreates the video element on every individual test case. This prevents a random failure that was showing up. The previous load was not finishing before the next one started, so the networkState was not EMPTY, causing an abort event to fire when the load for the next test came in, which screwed things up.
Attachment #361199 - Attachment is obsolete: true
Attachment #361458 - Flags: superreview?(roc)
Attachment #361458 - Flags: review?(chris.double)
Attachment #361458 - Flags: superreview?(roc) → superreview+

Updated

9 years ago
Attachment #361458 - Flags: review?(chris.double) → review+
Duplicate of this bug: 475868
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
(Assignee)

Comment 13

9 years ago
Comment on attachment 361458 [details] [diff] [review]
Patch v2

Yay. Bitrot. test_play.html which was added since last revision is now failing.
Attachment #361458 - Attachment is obsolete: true
(Assignee)

Comment 14

9 years ago
Created attachment 362163 [details] [diff] [review]
Patch v2.1 unbitrotten
[Checkin: Comment 15+16]

As v2, but with the recently added test fixed to not thrown an error in the new behaviour.
Attachment #362163 - Flags: superreview+
Attachment #362163 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs landing]
Attachment #362163 - Attachment description: Patch v2.1 unbitrotten → Patch v2.1 unbitrotten [Checkin: Comment 15]
Comment on attachment 362163 [details] [diff] [review]
Patch v2.1 unbitrotten
[Checkin: Comment 15+16]


http://hg.mozilla.org/mozilla-central/rev/252b7dc440db
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing] → [c-n: baking for 1.9.1]
Target Milestone: --- → mozilla1.9.2a1
Comment on attachment 362163 [details] [diff] [review]
Patch v2.1 unbitrotten
[Checkin: Comment 15+16]


http://hg.mozilla.org/mozilla-central/rev/c8cdcd82ba32
Missing file, copied from 'Patch v2'.

Please, check whether this file/code is up-to-date,
then attach a merged patch for 1.9.1.
Attachment #362163 - Attachment description: Patch v2.1 unbitrotten [Checkin: Comment 15] → Patch v2.1 unbitrotten [Checkin: Comment 15+16]
(Assignee)

Comment 17

9 years ago
Created attachment 362492 [details] [diff] [review]
Patch merged for 1.9.1

Patch for 1.9.1 branch. Note, this does not apply cleanly to 1.9.1. According to the whiteboard tags, the following video/audio bugs have been pushed to m-c but are still waiting to go into 1.9.1:

http://tinyurl.com/c5hffc

These probably need to be pushed to 1.9.1 before this patch will apply cleanly.

I used this query to generate that list:

http://tinyurl.com/as5r8u
Attachment #362492 - Flags: superreview+
Attachment #362492 - Flags: review+
Yeah. Serge: when you push patches to 1.9.1 (which I am very grateful for), please make sure you push in the order in which they landed in mozilla-central.
(In reply to comment #17)
> http://tinyurl.com/c5hffc

(In reply to comment #18)
> please make sure you push in the order in which they landed in mozilla-central.

I don't know (and won't look for) what this correct order is:
please update the whiteboard of these bugs with the needed details.
(Or (preferably !?) don't ask for c-n until this can actually be done.)
Fair enough. I have a script that will take a list of changesets and sort them into correct order which I use to do this.
Keywords: checkin-needed
Whiteboard: [c-n: baking for 1.9.1] → [needs 191 landing]
Blocks: 478652

Comment 21

9 years ago
I think this created a regression  see.
Bug 478805 -  Changing VIDEO.src dont change the actual source file being played.

Comment 22

9 years ago
See bug 478805 comment 1. It's not a regression.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9a4041b5740e
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6f6d5aace10e too
You need to log in before you can comment on or make changes to this bug.