[Webkit-unassigned] [Bug 83282] Web Inspector: Allow inspection of Web Socket Frames

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=83282





--- Comment #31 from Pavel Feldman <pfeldman at chromium.org>  2012-04-20 02:38:51 PST ---
(From update of attachment 137969)
View in context: https://bugs.webkit.org/attachment.cgi?id=137969&action=review

Few nits and it is good to go.

> Source/WebCore/inspector/front-end/NetworkRequest.js:810
> +    /**

You could narrow the API surface down via introducing single
frames() method.

> Source/WebCore/inspector/front-end/NetworkRequest.js:830
> +    getFrame: function(position)

WebKit guidelines discourages use of get prefixes. frame: function(position). I would strongly encourage you to simply expose frames: array here even if it returns live collection.

> Source/WebCore/inspector/front-end/NetworkRequest.js:863
> +            this._frames.splice(0,10);

Space before 10.

> Source/WebCore/inspector/front-end/ResourceWebSocketFrameView.js:67
> +        dateCell.innerHTML = String.sprintf("%s %s", (payload.sent ? "→" : (payload.errorMessage ? "" : "←")), date.toISOString());

It does not really matter whether this is user data or not, you should only use innerHTML = for the (a) sanitized (b) HTML content. Otherwise it instantiates HTML parser, works slowly and results in vulnerabilities. You should look up \u symbol codes for the arrows and append them to your text content.

> Source/WebCore/inspector/front-end/ResourceWebSocketFrameView.js:73
> +            spanCell.innerText = payload.errorMessage;

innerText is not standard, uses weird semantics. You want .textContent = here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.