Bug 185353 - Text track cue logging should include cue text
Summary: Text track cue logging should include cue text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-05 10:39 PDT by Eric Carlson
Modified: 2018-05-08 10:20 PDT (History)
11 users (show)

See Also:

Attachments
Proposed patch (4.22 KB, patch)
2018-05-05 15:10 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (4.52 KB, patch)
2018-05-05 20:33 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch for landing. (5.12 KB, patch)
2018-05-07 08:09 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Only log cue text in debug build (3.22 KB, patch)
2018-05-07 14:34 PDT, Eric Carlson 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2018-05-05 10:39:04 PDT 
Text track cue logging should include cue text
Comment 1 Radar WebKit Bug Importer 2018-05-05 10:39:30 PDT 
<rdar://problem/40003565>
Comment 2 Eric Carlson 2018-05-05 15:10:41 PDT 
Created attachment 339657 [details]
Proposed patch
Comment 3 Eric Carlson 2018-05-05 20:33:58 PDT 
Created attachment 339667 [details]
Patch
Comment 4 youenn fablet 2018-05-06 21:02:40 PDT 
Comment on attachment 339667 [details]
Patch

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

> Source/WebCore/html/track/VTTCue.h:180
> +    void toJSON(JSON::Object&) const final;

Could be made private maybe?

> Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h:123
> +    object->setString("text", m_content);

ASCIILiteral?

> Source/WebCore/platform/graphics/iso/ISOVTTCue.cpp:117
> +    object->setString(ASCIILiteral("text"), m_cueText);

Should it be "cueText"? Similarly for "text" -> "content" above maybe?
Comment 5 Eric Carlson 2018-05-07 08:09:59 PDT 
Created attachment 339720 [details]
Patch for landing.
Comment 6 WebKit Commit Bot 2018-05-07 08:55:02 PDT 
Comment on attachment 339720 [details]
Patch for landing.

Clearing flags on attachment: 339720

Committed r231438: <https://trac.webkit.org/changeset/231438>
Comment 7 Eric Carlson 2018-05-07 14:34:01 PDT 
Created attachment 339753 [details]
Only log cue text in debug build
Comment 8 Brent Fulgham 2018-05-08 09:53:18 PDT 
Comment on attachment 339753 [details]
Only log cue text in debug build

r=me
Comment 9 WebKit Commit Bot 2018-05-08 10:20:14 PDT 
Comment on attachment 339753 [details]
Only log cue text in debug build

Clearing flags on attachment: 339753

Committed r231490: <https://trac.webkit.org/changeset/231490>
Comment 10 WebKit Commit Bot 2018-05-08 10:20:16 PDT 
All reviewed patches have been landed.  Closing bug.