[webkit-reviews] review denied: [Bug 64601] Web Inspector: implement import/export for timeline data. : [Attachment 101294] [patch] fifth version with extracted model code.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 64601: Web Inspector: implement import/export for timeline data.
https://bugs.webkit.org/show_bug.cgi?id=64601

Attachment 101294: [patch] fifth version with extracted model code.
https://bugs.webkit.org/attachment.cgi?id=101294&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=101294&action=review


> LayoutTests/inspector/timeline/timeline-load.html:9
> +    if (window.layoutTestController)

remove?

> LayoutTests/inspector/timeline/timeline-load.html:40
> +	   if (JSON.stringify(data) == JSON.stringify(saveData))

InspectorTest.addResult(JSON.stringify(saveData));

> Source/WebCore/inspector/front-end/TimelinePanel.js:224
> +    _createFileSelector: function()

Can this be done lazily?

> Source/WebCore/inspector/front-end/TimelinePanel.js:240
> +	   contextMenu.appendItem(WebInspector.UIString("&Export Timeline
data..."), this._exportToFile.bind(this));

Use ellipsis character

> Source/WebCore/inspector/front-end/TimelinePanel.js:1253
> +		  
WebInspector.error(WebInspector.UIString('Timeline.importFromFile: File "%s"
not found.', file.name));

There is no WebInspector.error.

> Source/WebCore/inspector/front-end/TimelinePanel.js:1274
> +	   var date = String.sprintf("%4d%2d%2dT%2d%2d", now.getFullYear(),
now.getMonth() + 1, now.getDate(), now.getHours(), now.getMinutes());

You can move this to utilities.js (Date.prototype.toRFC2445)