Summary: | Implement WebVTT VTTCue region attribute | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Pieters (:zcorpan) <zcorpan> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, frankolivier, glenn, gyuyoung.kim, jer.noble, kondapallykalyan, pdr, philipj, sergio, silviapf, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Safari Technology Preview | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Simon Pieters (:zcorpan)
2017-02-22 03:16:31 PST
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 (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 Also please note that there is a :cue-region and :cue-region(id) pseudo-class to implement https://github.com/w3c/webvtt/issues/355 Created attachment 416044 [details]
Patch
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
Committed r270738: <https://trac.webkit.org/changeset/270738> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416099 [details]. |