[webkit-reviews] review denied: [Bug 50553] Web Inspector: make script view editable : [Attachment 76046] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Adaikin
<aandrey at google.com>'s request for review:
Bug 50553: Web Inspector: make script view editable
https://bugs.webkit.org/show_bug.cgi?id=50553

Attachment 76046: Patch
https://bugs.webkit.org/attachment.cgi?id=76046&action=review

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

Few nits to fix and it is good to land.

> WebCore/inspector/front-end/TextViewer.js:58
> +    this._linesContainerElement.addEventListener("DOMCharacterDataModified",
this._handleDOMCharacterDataModified.bind(this), false);

You should only add those in case of editable mode.

> WebCore/inspector/front-end/TextViewer.js:94
> +	       this._linesContainerElement.setAttribute("contentEditable",
!!this._editCallback);

this._linesContainerElement.contentEditable = !!this._editCallback;

> WebCore/inspector/front-end/TextViewer.js:241
>      _handleKeyDown: function()

I think you should conditionally add listeners instead.

> WebCore/inspector/front-end/TextViewer.js:272
>      _handleDoubleClick: function(e)

ditto