Closed Bug 1276831 Opened 8 years ago Closed 8 years ago

[webvtt] Let VTTCue's position return double or AutoKeyword

Categories

(Core :: Audio/Video: Playback, defect)

Product:
Core
Component:
Audio/Video: Playback
Version:
Other Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox50 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.
58 bytes, text/x-review-board-request
smaug
: review+
rillian
: review+
Details
Bug 1276831 - part2 : modify test.
58 bytes, text/x-review-board-request
rillian
: review+
Details
Bug 1276831 - part3 : modify vtt.jsm.
58 bytes, text/x-review-board-request
rillian
: review+
Details
Bug 1276831 - part4 : modify web-platform-test.
58 bytes, text/x-review-board-request
rillian
: review+
Details 
https://w3c.github.io/webvtt/#dom-vttcue-position

Now the position getter returns "long", but it's wrong.
Attachment #8760130 - Flags: review?(giles)
Attachment #8760130 - Flags: review?(bugs)
Attachment #8760131 - Flags: review?(giles)
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

https://reviewboard.mozilla.org/r/57848/#review54734

r+ for the .webidl
Attachment #8760130 - Flags: review?(bugs) → review+
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57848/diff/1-2/
Comment on attachment 8760131 [details]
Bug 1276831 - part2 : modify test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57850/diff/1-2/
Comment on attachment 8760131 [details]
Bug 1276831 - part2 : modify test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57850/diff/2-3/
Sorry, Alastair. I've not been feeling well this week. I expect to be able to get to your reviews tomorrow or Friday.
Attachment #8760130 - Flags: review?(giles) → review+
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

https://reviewboard.mozilla.org/r/57848/#review56060
Attachment #8760131 - Flags: review?(giles) → review+
Attachment #8761054 - Flags: review?(giles) → review+
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57848/diff/2-3/
Comment on attachment 8760131 [details]
Bug 1276831 - part2 : modify test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57850/diff/3-4/
Comment on attachment 8761054 [details]
Bug 1276831 - part3 : modify vtt.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58410/diff/1-2/
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/568a08748f67
part1 : VTTCue's position should be double or auto keyword. r=rillian,smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7373932c9e56
part2 : modify test. r=rillian
https://hg.mozilla.org/integration/mozilla-inbound/rev/79323e71de55
part3 : modify vtt.jsm. r=rillian
Backed out for failure in web-platform-test webvtt/interfaces.html. Also had to backout bug 1276130 to prevent failing hunks during the backout.

bug 1276831:
https://hg.mozilla.org/integration/mozilla-inbound/rev/017c7d55f22fc67346a9ef2ec45bd33ae88d8c46
https://hg.mozilla.org/integration/mozilla-inbound/rev/803228efe10858012ab159a694d913de136bc9e2
https://hg.mozilla.org/integration/mozilla-inbound/rev/76cffbe704e27cf82cf062aa2ef5f385966224ed

bug 1276130:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58641c44fc7f5f03ede954ad96eac80d66978cdd
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b87112d3f96715bac0c8cab6acbad0276e59ca4
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf15e558b3b9c28625b7d7b78f379bfd6ad3dcc1

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=79323e71de55371e66aff9a04adb3eff1235c26d
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=30101337&repo=mozilla-inbound

12:10:46     INFO - TEST-PASS | /webvtt/interfaces.html | VTTCue interface: new VTTCue(0, 1, "foo") must inherit property "vertical" with the proper type (1) 
12:10:46     INFO - TEST-PASS | /webvtt/interfaces.html | VTTCue interface: new VTTCue(0, 1, "foo") must inherit property "snapToLines" with the proper type (2) 
12:10:46     INFO - TEST-FAIL | /webvtt/interfaces.html | VTTCue interface: new VTTCue(0, 1, "foo") must inherit property "line" with the proper type (3) - Unrecognized type [object Object],[object Object]
12:10:46     INFO - TEST-PASS | /webvtt/interfaces.html | VTTCue interface: new VTTCue(0, 1, "foo") must inherit property "lineAlign" with the proper type (4) 
12:10:46     INFO - TEST-UNEXPECTED-FAIL | /webvtt/interfaces.html | VTTCue interface: new VTTCue(0, 1, "foo") must inherit property "position" with the proper type (5) - assert_equals: expected "number" but got "string"
12:10:46     INFO - IdlArray.prototype.assert_type_is@http://web-platform.test:8000/resources/idlharness.js:479:13
12:10:46     INFO - IdlInterface.prototype.test_interface_of/<@http://web-platform.test:8000/resources/idlharness.js:1526:29
12:10:46     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1398:20
12:10:46     INFO - test@http://web-platform.test:8000/resources/testharness.js:496:9
12:10:46     INFO - IdlInterface.prototype.test_interface_of@http://web-platform.test:8000/resources/idlharness.js:1495:13
12:10:46     INFO - IdlInterface.prototype.test_object@http://web-platform.test:8000/resources/idlharness.js:1407:9
12:10:46     INFO - IdlArray.prototype.test/<@http://web-platform.test:8000/resources/idlharness.js:364:17
12:10:46     INFO - IdlArray.prototype.test@http://web-platform.test:8000/resources/idlharness.js:362:13
12:10:46     INFO - window.onload@http://web-platform.test:8000/webvtt/interfaces.html:61:2
12:10:46     INFO - EventHandlerNonNull*@http://web-platform.test:8000/webvtt/interfaces.html:55:1
Flags: needinfo?(alwu)
The idl-test-harness can't detect the multiple types in webidl [1], so it doesn't know about the type like "double or AutoKeyword".
The same problem also happen in VTTCue's line property, and we have already expected its fail.
Therefore, we should expect this fail before we fix the problem of idl-test-harness.

[1] http://searchfox.org/mozilla-central/source/dom/imptests/idlharness.js#375

Review commit: https://reviewboard.mozilla.org/r/59214/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59214/
Attachment #8762676 - Flags: review?(giles)
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57848/diff/3-4/
Comment on attachment 8760131 [details]
Bug 1276831 - part2 : modify test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57850/diff/4-5/
Comment on attachment 8761054 [details]
Bug 1276831 - part3 : modify vtt.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58410/diff/2-3/
Flags: needinfo?(alwu)
Comment on attachment 8762676 [details]
Bug 1276831 - part4 : modify web-platform-test.

https://reviewboard.mozilla.org/r/59214/#review56358

unforunate, but still moves us forward. Can you please file a bug for the idl test harness?
Attachment #8762676 - Flags: review?(giles) → review+
See Also: → 1280275
Comment on attachment 8760130 [details]
Bug 1276831 - part1 : VTTCue's position should be double or auto keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57848/diff/4-5/
Comment on attachment 8760131 [details]
Bug 1276831 - part2 : modify test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57850/diff/5-6/
Comment on attachment 8761054 [details]
Bug 1276831 - part3 : modify vtt.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58410/diff/3-4/
Comment on attachment 8762676 [details]
Bug 1276831 - part4 : modify web-platform-test.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59214/diff/1-2/
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4630640ca42
part1 : VTTCue's position should be double or auto keyword. r=rillian,smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3302f0b398b1
part2 : modify test. r=rillian
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bc569066f9d
part3 : modify vtt.jsm. r=rillian
https://hg.mozilla.org/integration/mozilla-inbound/rev/2846dfd72c54
part4 : modify web-platform-test. r=rillian
Apparently this breaks our vtt.js polyfill.  See https://github.com/mozilla/vtt.js/issues/354

Is that intended?  Is that polyfill still maintained?
Flags: needinfo?(giles)
https://github.com/mozilla/vtt.js is another version of the in-tree vtt.jsm. I haven't been maintaining it, but it seems it needs these changes applied there as well.
Flags: needinfo?(giles)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: