Bug 33272

Summary:

'abort' event still fired as a progress event

Product:

WebKit

Reporter:

Eric Carlson <eric.carlson>

Component:

Media

Assignee:

Eric Carlson <eric.carlson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dbates, eric, gustavo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

OS X 10.5

Attachments:

Description	Flags
Proposed patch	none
Proposed patch	none

Eric Carlson

Reported 2010-01-06 15:31:37 PST

https://bugs.webkit.org/show_bug.cgi?id=30513 should have changed all events fired by the HTMLMediaElement to regular events, but 'abort' was missed.

Attachments
Proposed patch (4.86 KB, patch) 2010-01-06 15:34 PST, Eric Carlson	no flags	Details Formatted Diff Diff
Proposed patch (12.52 KB, patch) 2010-01-06 22:15 PST, Eric Carlson	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric Carlson

Comment 1 2010-01-06 15:34:11 PST

Created attachment 46002 [details] Proposed patch

WebKit Review Bot

Comment 2 2010-01-06 15:34:38 PST

style-queue ran check-webkit-style on attachment 46002 [details] without any errors.

Darin Adler

Comment 3 2010-01-06 16:01:07 PST

Comment on attachment 46002 [details] Proposed patch Is there some test that covers this? Could you make a test that does? I'm tempted to set review+ but not sure if I should without a test covering the change.

Eric Seidel (no email)

Comment 4 2010-01-06 16:01:50 PST

Comment on attachment 46002 [details] Proposed patch Can we test for lack of an abort event? Marking cq- since this couldn't be committed as-is due to the second OOPS even if you had wanted to cq it (which I doubt).

Eric Carlson

Comment 5 2010-01-06 17:21:07 PST

media/video-error-abort.html *used* to test the abort event, but it became timing dependent when we stopped firing media events synchronously [1] so it was added to the skipped lists [2]. I will update it for the current behavior, have it to check the abort event, and move it to http/tests/media so it can use the slow loading cgi and actually work reliably. [1] https://bugs.webkit.org/show_bug.cgi?id=24588 [2] rdar://6710625

Eric Carlson

Comment 6 2010-01-06 22:15:02 PST

Created attachment 46022 [details] Proposed patch With layout test.

WebKit Review Bot

Comment 7 2010-01-06 22:16:04 PST

style-queue ran check-webkit-style on attachment 46022 [details] without any errors.

Maciej Stachowiak

Comment 8 2010-01-07 02:32:32 PST

Comment on attachment 46022 [details] Proposed patch r=me

Eric Carlson

Comment 9 2010-01-07 08:15:12 PST

http://trac.webkit.org/changeset/52923

Eric Seidel (no email)

Comment 10 2010-01-07 11:29:56 PST

This broke the Leopard Release bot: http://build.webkit.org/results/Leopard%20Intel%20Release%20(Tests)/r52923%20(9069)/http/tests/media/video-error-abort-pretty-diff.html

Eric Seidel (no email)

Comment 11 2010-01-07 11:57:11 PST

This also broke the Gtk and Qt builders. I assume because they had skipped this test previously.

Eric Seidel (no email)

Comment 12 2010-01-07 11:58:30 PST

Looks like it broken windows too: http://build.webkit.org/results/Windows%20Release%20(Tests)/r52923%20(7673)/results.html http://build.webkit.org/results/Windows%20Debug%20(Tests)/r52923%20(8233)/results.html

Eric Carlson

Comment 13 2010-01-07 12:26:21 PST

The svn:executable flag wasn't set on the cgi, setting it fixed Windows and Leopard Release (http://trac.webkit.org/changeset/52935). I forgot to add the new test to the GTK Skipped list. Eric lied about me breaking the Qt build (this time).

Eric Seidel (no email)

Comment 14 2010-01-07 12:28:51 PST

Thank you for the quick fix. I am a liar. :(

Note You need to log in before you can comment on or make changes to this bug.