[webkit-reviews] review denied: [Bug 105797] Web Inspector: refactor DefaultTextEditor's private methods : [Attachment 180793] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Andrey Lushnikov
<lushnikov at chromium.org>'s request for review:
Bug 105797: Web Inspector: refactor DefaultTextEditor's private methods
https://bugs.webkit.org/show_bug.cgi?id=105797

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

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


> Source/WebCore/inspector/front-end/DefaultTextEditor.js:414
> +	   this._mainPanel.handleTextInput(e);

Just add this listener within the main panel.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:450
>	       this._delegate.populateTextAreaContextMenu(contextMenu, target
&& target.lineNumber);

You should instead pass event.target here
(this._mainPanel.handleContextMenu(event);)

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:458
> +	   var firstVisibleLineNumber =
this._mainPanel.findFirstVisibleLineNumber(visibleFrom);

lineNumberAtOffset(visibleFrom)

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:491
> +	   this._mainPanel.setLastSelection(textRange);

I'd rather introduce mainPanel.setSelection that would do this all.
_restoreSelection does not need to be public then.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:590
> +	   this.freeCachedElements();

You already have willHide, free cached elements from there.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1030
> +    this.element.addEventListener("scroll", this.scroll.bind(this), false);

Just create this.element in TextEditorChunkedPanel, add listener there.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1307
> +	   return this._textEditor.totalHeight(this._expandedLineRows[0],
this._expandedLineRows[this._expandedLineRows.length - 1]);

rename textEditor to chunkedPanel