Bug 109821

Summary: TextTrackCue Extension for WebVTT Regions
Product: WebKit Reporter: Victor Carbune <vcarbune>
Component: MediaAssignee: Victor Carbune <vcarbune>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, buildbot, dglazkov, eric.carlson, esprehn+autocc, feature-media-reviews, glenn, jer.noble, ojan.autocc, peter+ews, rniwa, silviapf, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 109570    
Attachments:
Description Flags
Patch and Test
none
Updated Patch
none
Updated Patch none

Description Victor Carbune 2013-02-14 05:53:35 PST
The TextTrackCue object and settings are extended with:
Parsing (support for region identifier) and JavaScript (regionId attribute)
Comment 1 Glenn Adams 2013-02-14 07:01:42 PST
propose to the W3C for standardization, and, if they accept and specify, then you may reopen this bug
Comment 2 Eric Carlson 2013-02-14 08:51:12 PST
As noted in the master bug (#109570), there is a proposed solution [1] [2] which has had a great deal of discussion in the text community group mailing list (eg. [3]).

[1] https://dvcs.w3.org/hg/text-tracks/raw-file/default/608toVTT/region.html
[2] http://www.w3.org/community/texttracks/wiki/MultiCueBox
[3] http://lists.w3.org/Archives/Public/public-texttracks/2012Dec/0000.html
Comment 3 Victor Carbune 2013-04-01 08:43:46 PDT
Created attachment 195975 [details]
Patch and Test
Comment 4 WebKit Review Bot 2013-04-01 08:55:16 PDT
Comment on attachment 195975 [details]
Patch and Test

Attachment 195975 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17390020
Comment 5 Eric Carlson 2013-04-01 08:57:59 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=195975&action=review

> Source/WebCore/html/track/TextTrackCue.cpp:208
> +    , m_regionId(emptyString())

This isn't necessary, the compiler automatically calls the default constructor.

> Source/WebCore/html/track/TextTrackCue.cpp:1105
> +    if (m_regionId == emptyString())

if (!m_regionId.isEmpty())
Comment 6 WebKit Review Bot 2013-04-01 09:01:58 PDT
Comment on attachment 195975 [details]
Patch and Test

Attachment 195975 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17230972
Comment 7 Peter Beverloo (cr-android ews) 2013-04-01 09:04:51 PDT
Comment on attachment 195975 [details]
Patch and Test

Attachment 195975 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17230970
Comment 8 Build Bot 2013-04-01 09:23:14 PDT
Comment on attachment 195975 [details]
Patch and Test

Attachment 195975 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17337455
Comment 9 Victor Carbune 2013-04-01 09:34:49 PDT
Created attachment 195982 [details]
Updated Patch
Comment 10 Victor Carbune 2013-04-01 09:36:28 PDT
(In reply to comment #5)
> View in context: https://bugs.webkit.org/attachment.cgi?id=195975&action=review
> 
> > Source/WebCore/html/track/TextTrackCue.cpp:208
> > +    , m_regionId(emptyString())
> 
> This isn't necessary, the compiler automatically calls the default constructor.

Done.

> > Source/WebCore/html/track/TextTrackCue.cpp:1105
> > +    if (m_regionId == emptyString())
> 
> if (!m_regionId.isEmpty())

Done.
Comment 11 WebKit Review Bot 2013-04-01 09:41:10 PDT
Comment on attachment 195982 [details]
Updated Patch

Attachment 195982 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17337462
Comment 12 WebKit Review Bot 2013-04-01 09:44:21 PDT
Comment on attachment 195982 [details]
Updated Patch

Attachment 195982 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17336410
Comment 13 Peter Beverloo (cr-android ews) 2013-04-01 09:50:12 PDT
Comment on attachment 195982 [details]
Updated Patch

Attachment 195982 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17336412
Comment 14 Victor Carbune 2013-04-01 10:10:09 PDT
Created attachment 195985 [details]
Updated Patch
Comment 15 WebKit Review Bot 2013-04-01 13:09:12 PDT
Comment on attachment 195985 [details]
Updated Patch

Clearing flags on attachment: 195985

Committed r147355: <http://trac.webkit.org/changeset/147355>
Comment 16 WebKit Review Bot 2013-04-01 13:09:22 PDT
All reviewed patches have been landed.  Closing bug.