Closed Bug 1420357 Opened 2 years ago Closed 2 years ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bechen, Assigned: bechen)

References

Details

Attachments

(2 files)

No description provided.
https://www.w3.org/TR/webvtt1/#webvtt-cue-position-alignment
The default value of positionAlign is "auto".
Priority: -- → P3
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.
Attachment #8932031 - Flags: review?(giles) → review+
Comment on attachment 8933163 [details]
Bug 1420357 - The default value of positionAlign is "auto".

https://reviewboard.mozilla.org/r/204124/#review209680
Attachment #8933163 - Flags: review?(giles) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/af5a15ecd168
https://hg.mozilla.org/mozilla-central/rev/92134e1bfca1
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.