Bug 91714

Summary: Web Inspector: Timeline: forward compatibility for load.
Product: WebKit Reporter: eustas.bug
Component: Web Inspector (Deprecated)Assignee: eustas.bug
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, caseq, eustas.bug, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Demo screenshot
none
Patch
none
Patch
none
Patch none

Description eustas.bug 2012-07-18 22:55:26 PDT
Created attachment 153182 [details]
Demo screenshot

When you try to load timeline data with record types unknown in current version, exception occurs.

Fix: accept record of unrecognized types and render them as "unknown".
Comment 1 eustas.bug 2012-07-18 23:13:55 PDT
Created attachment 153184 [details]
Patch
Comment 2 Vsevolod Vlasov 2012-07-19 02:13:16 PDT
Comment on attachment 153184 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:66
> +        var recordTypes = WebInspector.TimelineModel.RecordType;

Maybe extract _initRecordStyles() ?

> LayoutTests/inspector/timeline/timeline-load-incompatible.html:43
> +Tests the Timeline save/load of unknown (incompatible) record types.

Please add a link to the bug below.
Comment 3 eustas.bug 2012-07-19 03:17:31 PDT
Comment on attachment 153184 [details]
Patch

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

>> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:66
>> +        var recordTypes = WebInspector.TimelineModel.RecordType;
> 
> Maybe extract _initRecordStyles() ?

Done.

>> LayoutTests/inspector/timeline/timeline-load-incompatible.html:43
>> +Tests the Timeline save/load of unknown (incompatible) record types.
> 
> Please add a link to the bug below.

Done.
Comment 4 eustas.bug 2012-07-19 03:18:13 PDT
Created attachment 153217 [details]
Patch
Comment 5 Andrey Kosyakov 2012-07-19 03:25:18 PDT
Comment on attachment 153217 [details]
Patch

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

> Source/WebCore/English.lproj/localizedStrings.js:219
> +localizedStrings["Unknown: "] = "Unknown: ";

Unknown: %s

> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:115
> +            title: WebInspector.UIString("Unknown: ") + WebInspector.UIString(record.type),

So where would we get a UIString for unknown records? ;-)
Comment 6 eustas.bug 2012-07-19 04:19:28 PDT
Comment on attachment 153217 [details]
Patch

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

>> Source/WebCore/English.lproj/localizedStrings.js:219
>> +localizedStrings["Unknown: "] = "Unknown: ";
> 
> Unknown: %s

Fixed.

>> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:115
>> +            title: WebInspector.UIString("Unknown: ") + WebInspector.UIString(record.type),
> 
> So where would we get a UIString for unknown records? ;-)

Hope never dies =)
Fixed.
Comment 7 eustas.bug 2012-07-19 04:28:19 PDT
Created attachment 153225 [details]
Patch
Comment 8 Andrey Kosyakov 2012-07-20 01:59:42 PDT
Comment on attachment 153225 [details]
Patch

LGTM
Comment 9 WebKit Review Bot 2012-07-20 02:41:50 PDT
Comment on attachment 153225 [details]
Patch

Clearing flags on attachment: 153225

Committed r123198: <http://trac.webkit.org/changeset/123198>
Comment 10 WebKit Review Bot 2012-07-20 02:41:55 PDT
All reviewed patches have been landed.  Closing bug.