1420357 - [webvtt] fix wpt test settings-position.html.

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Comment 1

[Benjamin Chen [:bechen]]

https://www.w3.org/TR/webvtt1/#webvtt-cue-position-alignment
The default value of positionAlign is "auto".

Comment 2

[Benjamin Chen [:bechen]]

Attachment: Bug 1420357 - fix postion testcase.

1. The defaul valus of position is "auto".
2. follow the spec to parse the position value.

Review commit: https://reviewboard.mozilla.org/r/203104/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203104/

Comment 3

[Ralph Giles (:rillian)]

Comment on attachment 8932031 [details]
Bug 1420357 - fix postion testcase.

https://reviewboard.mozilla.org/r/203104/#review208622

::: commit-message-d386a:3
(Diff revision 1)
> +Bug 1420357 - fix postion testcase. r=rillian
> +
> +1. The defaul valus of position is "auto".

Two typos:

`1. The default value of position is "auto".`

::: dom/media/webvtt/vtt.jsm:228
(Diff revision 1)
>          case "position":
>            vals = v.split(",");
> -          settings.percent(k, vals[0]);
> +          if (settings.percent(k, vals[0])) {
> -          if (vals.length === 2) {
> +            if (vals.length === 2) {
> -            settings.alt("positionAlign", vals[1], ["line-left", "center", "line-right", "auto"]);
> +              if (!settings.alt("positionAlign", vals[1], ["line-left", "center", "line-right"])) {
> +                settings.del(k);

So if `auto` occurs here, the key is removed, and then when the property is copie to `cue.position` the default value of `auto` is subsitited again.

Tricky to read, but it works. Maybe a comment would be good.

Comment 4

[Benjamin Chen [:bechen]]

Comment on attachment 8932031 [details]
Bug 1420357 - fix postion testcase.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203104/diff/1-2/

Comment 5

[Benjamin Chen [:bechen]]

Attachment: Bug 1420357 - The default value of positionAlign is "auto".

Review commit: https://reviewboard.mozilla.org/r/204124/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/204124/

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8933163 [details]
Bug 1420357 - The default value of positionAlign is "auto".

https://reviewboard.mozilla.org/r/204124/#review209680

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af5a15ecd168
fix postion testcase. r=rillian
https://hg.mozilla.org/integration/autoland/rev/92134e1bfca1
The default value of positionAlign is "auto". r=rillian

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/af5a15ecd168
https://hg.mozilla.org/mozilla-central/rev/92134e1bfca1