Bug 168716 – Implement WebVTT VTTCue region attribute

• Component: Media
• Status: RESOLVED FIXED

[Simon Pieters (:zcorpan)]

Spec http://w3c.github.io/webvtt/#dom-vttcue-region
Test http://w3c-test.org/webvtt/api/VTTCue/region.html

This was implemented in Chromium in https://bugs.chromium.org/p/chromium/issues/detail?id=690014#c3

[Simon Pieters (:zcorpan)]

I suppose it makes sense to update the WebVTT region-related API changes all at once; see https://github.com/w3c/web-platform-tests/pull/4997

In historical.html, tests that fail in Safari TP:

VTTCue regionId member must be nuked
TextTrack regions member must be nuked
TextTrack addRegion member must be nuked
TextTrack removeRegion member must be nuked
VTTRegion track member must be nuked
VTTRegion id member must be nuked

(Note: VTTRegion id is very likely to be added back to the spec.)


See also IDL tests added in https://github.com/w3c/web-platform-tests/pull/4992

[Simon Pieters (:zcorpan)]

(In reply to Simon Pieters from comment #1)
> VTTRegion id member must be nuked
> 
> (Note: VTTRegion id is very likely to be added back to the spec.)

id was re-introduced in https://github.com/w3c/webvtt/pull/349
test at https://github.com/w3c/web-platform-tests/pull/6885

[Silvia Pfeiffer]

Also please note that there is a :cue-region and :cue-region(id) pseudo-class to implement https://github.com/w3c/webvtt/issues/355

[Radar WebKit Bug Importer]

<rdar://problem/71815641>

Created attachment 416044 [details]
Patch

[Eric Carlson]

Comment on attachment 416044 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416044&action=review

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:97
> +        

Nit: don't need this new whitespace

> Source/WebCore/html/track/VTTCue.cpp:199
> +    

Ditto.

> Source/WebCore/html/track/VTTCue.cpp:584
> +        // return Exception { SyntaxError };

Did you mean to leave this commented out?

> Source/WebCore/html/track/VTTCue.cpp:1063
> +        if (m_region) {
> +            if (m_displayTree)
> +                m_region->willRemoveTextTrackCueBox(m_displayTree.get());
>          }

Nit: this could be simplified to `if (m_region && m_displayTree) ...`

> Source/WebCore/html/track/VTTCue.h:210
> +//    bool linePositionIsAuto() const;

Should this be deleted?

> Source/WebCore/rendering/RenderVTTCue.cpp:62
> +    

Nit: whitespace

> Source/WebCore/rendering/RenderVTTCue.cpp:391
> +
> +

Nit: extra blank line

Created attachment 416099 [details]
Patch

[EWS]

Committed r270738: <https://trac.webkit.org/changeset/270738>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416099 [details].