[webkit-reviews] review denied: [Bug 178185] Web Inspector: Canvas tab: show detailed status during canvas recording : [Attachment 326266] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has denied Devin Rousso <webkit at devinrousso.com>'s
request for review:
Bug 178185: Web Inspector: Canvas tab: show detailed status during canvas
recording
https://bugs.webkit.org/show_bug.cgi?id=178185

Attachment 326266: Patch

https://bugs.webkit.org/attachment.cgi?id=326266&action=review




--- Comment #10 from Brian Burg <bburg at apple.com> ---
Comment on attachment 326266
  --> https://bugs.webkit.org/attachment.cgi?id=326266
Patch

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

r- because backend changes don't seem to be included. That would also imply new
tests or improving existing ones to log the progress events. =)

> Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:1016
> +localizedStrings["Waiting for frames..."] = "Waiting for frames...";

We typically use '…' such as in "Edit Breakpoint…" Please fix that.

> Source/WebInspectorUI/UserInterface/Protocol/CanvasObserver.js:50
> +    recordingProgress(canvasId, frames, bufferUsed)

You need to include the backend and protocol changes for this in the patch. And
add tests...

> Source/WebInspectorUI/UserInterface/Views/CanvasContentView.js:271
> +	   this._recordingProgressElement.textContent = WI.UIString("Waiting
for frames...");

Nit: …