949642 - [WebVTT] Implement VTTCue::LineAlign

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Rick Eyre (:reyre)]

This is a new property on the VTTCue idl.

Comment 1

[Rick Eyre (:reyre)]

Attachment: Bug 949642 - Implement VTTCue::LineAlign r=rillian

I don't quite think it makes sense to throw our own SYNTAX_ERRORS. Instead we could define a different Align setting with only the values start, middle, and end. Then the WebIDL bindings would do the checking and throwing for us. This isn't in the spec yet, though.

Comment 2

[Ralph Giles (:rillian)]

Comment on attachment 8347253 [details] [diff] [review]
Bug 949642 - Implement VTTCue::LineAlign r=rillian

Review of attachment 8347253 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks for writing tests!

::: content/media/test/test_texttrackcue.html
@@ +91,5 @@
> +        exceptionHappened = true;
> +        is(e.name, "SyntaxError", "Should have thrown SyntaxError.");
> +      }
> +      ok(exceptionHappened, "Exception should have happened.");
> +

I'd also try 'MIDDLE' or some other variant to verify the case-sensitivity.

Comment 3

[Rick Eyre (:reyre)]

Attachment: v2: Implement VTTCue::LineAlign r=rillian

Thanks for review Ralph.

Added test to test case sensitivity by setting cue.lineAlign = "START".

Carrying forward r=rillian.

Try: https://tbpl.mozilla.org/?tree=Try&rev=dde4206fc7be

Comment 4

[Rick Eyre (:reyre)]

Try looks good.

Comment 5

[Rick Eyre (:reyre)]

Attachment: v3: Implement VTTCue::LineAlign r=rillian

Rebased to current. Carrying forward r=rillian.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df0855f26e4d

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/208aaf5579aa because something from the push this was in introduced a new intermittent failure on Android: https://tbpl.mozilla.org/php/getParsedLog.php?id=32309229&tree=Mozilla-Inbound

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/df0855f26e4d

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Merged backout to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/208aaf5579aa

Comment 10

[Rick Eyre (:reyre)]

Failure was caused by bug 921484. Has landed now. Try: https://tbpl.mozilla.org/?tree=Try&rev=262a49175fb5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e588fb2c675

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9e588fb2c675