Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

Intent: https://groups.google.com/a/chromium.org/d/topic/blink-dev/6-oPQN4lZ2o/discussion

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
Committed: https://crrev.com/00923f90277216a21263f16898fb404c4a5741c8
Cr-Commit-Position: refs/heads/master@{#392911}

[fs, 2016-05-04 13:59:56]

Description was changed from

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
==========

to

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
==========

[fs, 2016-05-04 15:15:02]

fs@opera.com changed reviewers:
+ davve@opera.com

[fs, 2016-05-04 15:15:02]

Quiz: How many lines do you need to change to change a default value?
Answer: ...

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/track/TextTrack.h (right):

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/track/TextTrack.h:126: AtomicString
invalidValueDefaultKind() const override { return metadataKeyword(); }
(After doing this fix I can't say that I'm too thrilled about this method - or
even pair of methods. Too overly generically side-effecty...)

[davve, 2016-05-06 14:56:15]

Code change looks good.

In https://bugzilla.mozilla.org/show_bug.cgi?id=1269712#c3 Boris raises a point
about advertising this change. Something for Blink also, through an intent to
implement?

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/track/TextTrack.h (right):

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/track/TextTrack.h:126: AtomicString
invalidValueDefaultKind() const override { return metadataKeyword(); }
On 2016/05/04 15:15:02, fs (ooo) wrote:
> (After doing this fix I can't say that I'm too thrilled about this method - or
> even pair of methods. Too overly generically side-effecty...)

Bringing the implementation closer in structure to
https://html.spec.whatwg.org/multipage/infrastructure.html#keywords-and-enume...
might be worthwhile.

[fs, 2016-05-09 11:58:17]

On 2016/05/06 at 14:56:15, davve wrote:
> Code change looks good.
> 
> In https://bugzilla.mozilla.org/show_bug.cgi?id=1269712#c3 Boris raises a
point about advertising this change. Something for Blink also, through an intent
to implement?

Seeing the Mozilla intent-thread it'll probably be good form to make on of our
own.

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/track/TextTrack.h (right):

https://codereview.chromium.org/1947033002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/track/TextTrack.h:126: AtomicString
invalidValueDefaultKind() const override { return metadataKeyword(); }
On 2016/05/06 at 14:56:15, David Vest wrote:
> On 2016/05/04 15:15:02, fs (ooo) wrote:
> > (After doing this fix I can't say that I'm too thrilled about this method -
or
> > even pair of methods. Too overly generically side-effecty...)
> 
> Bringing the implementation closer in structure to
https://html.spec.whatwg.org/multipage/infrastructure.html#keywords-and-enume...
might be worthwhile.

Yes... When this will be done will probably depend on the outcome of the above.
(I.e if we need to add code to measure this it'll probably be easier to
restructure first...)

[davve, 2016-05-11 08:01:27]

lgtm after API owner's approval:

https://groups.google.com/a/chromium.org/d/msg/blink-dev/6-oPQN4lZ2o/9gc2BzDW...

[fs, 2016-05-11 08:44:53]

Description was changed from

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
==========

to

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

Intent:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/6-oPQN4lZ2o/discus...

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
==========

[fs, 2016-05-11 08:53:30]

On 2016/05/11 at 08:01:27, davve wrote:
> lgtm after API owner's approval:
> 
>
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6-oPQN4lZ2o/9gc2BzDW...

Thanks, I just added a link to the intent thread to the description. Will
probably need to rebase before landing though. (Hopefully trivial though.)

[fs, 2016-05-11 10:37:50]

On 2016/05/11 at 08:53:30, fs wrote:
> On 2016/05/11 at 08:01:27, davve wrote:
> > lgtm after API owner's approval:
> > 
> >
https://groups.google.com/a/chromium.org/d/msg/blink-dev/6-oPQN4lZ2o/9gc2BzDW...
> 
> Thanks, I just added a link to the intent thread to the description. Will
probably need to rebase before landing though. (Hopefully trivial though.)

Rebased. media/track/track-kind.html had to be adjusted quite a bit more, but
nothing really controversial (still per what's mentioned in the description.)

[davve, 2016-05-11 11:28:40]

On 2016/05/11 10:37:50, fs wrote:
> Rebased. media/track/track-kind.html had to be adjusted quite a bit more, but
> nothing really controversial (still per what's mentioned in the description.)

still lgtm

[fs, 2016-05-11 11:29:31]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-05-11 11:29:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1947033002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1947033002/40001

[commit-bot: I haz the power, 2016-05-11 13:54:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-11 13:56:48]

Description was changed from

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

Intent:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/6-oPQN4lZ2o/discus...

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772
==========

to

==========
Change "invalid value default" for HTMLTrackElement 'kind' to "metadata"

Rename TrackBase::defaultKind() to invalidValueDefaultKind() to better
reflect its semantics. Also make sure that the "missing value default"
is set appropriately (in TextTrack constructors and on removal in
HTMLTrackElement parseAttribute) now that it differs from the "invalid
value default".

The test media/track/track-kind.html is adjusted so that it doesn't
check if a cue is displayed, since that depends on unspecified behavior
wrt how 'mode' changes when 'kind' does. (See comment in
TextTrack::setKind.)

The WPT tests will eventually get updated via Mozilla's automatic sync,
so adding expectations for now.
(Ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1269712)

Intent:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/6-oPQN4lZ2o/discus...

https://github.com/whatwg/html/issues/293
https://html.spec.whatwg.org/multipage/embedded-content.html#attr-track-kind

BUG=608772

Committed: https://crrev.com/00923f90277216a21263f16898fb404c4a5741c8
Cr-Commit-Position: refs/heads/master@{#392911}
==========

[commit-bot: I haz the power, 2016-05-11 13:56:49]

Patchset 3 (id:??) landed as
https://crrev.com/00923f90277216a21263f16898fb404c4a5741c8
Cr-Commit-Position: refs/heads/master@{#392911}