[webkit-reviews] review denied: [Bug 89461] Web Inspector: Design WebSockets panel : [Attachment 149746] Remove Opcode column

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Nikita Vasilyev
<me at elv1s.ru>'s request for review:
Bug 89461: Web Inspector: Design WebSockets panel
https://bugs.webkit.org/show_bug.cgi?id=89461

Attachment 149746: Remove Opcode column
https://bugs.webkit.org/attachment.cgi?id=149746&action=review

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


> Source/WebCore/ChangeLog:6
> +	   Use DataGrid to display the data.

I'd keep these details more dense (i.e. no blank line separators)

> Source/WebCore/inspector/front-end/ResourceWebSocketFrameView.js:58
> +	       row.data =
WebInspector.ResourceWebSocketFrameView.OpCodes[payload.opcode] + " (Opcode " +
payload.opcode + (payload.mask ? ", mask" : "") + ")";

You should use formatting methods on String.prototype for that (sprintf, etc.).
You also want to add new strings to the English.lproj.

> Source/WebCore/inspector/front-end/ResourceWebSocketFrameView.js:72
> +    0: "Continuation Frame",

I'd rather use a standard programming pattern for implementing it, i.e.

WebInspector.ResourceWebSocketFrameView.OpCodes = {
    ContinuationFrame: 0,
   ...
}

I would use it everywhere in the logic code and would choose the localized
string using "switch" upon UI creation.