A pre-work is needed for splitting classes from DefaultTextEditor.js into four files: DefaultTextEditor.js (DefaultTextEditor class), TextEditorMainPanel.js (TextEditorMainPanel and TextEditorMainChunk classes), TextEditorGutterPanel.js (TextEditorGutterPanel and TextEditorGutterChunk classes) and TextEditorChunkedPanel.js (TextEditorChunkedPanel class). Refactor DefaultTextEditor so that different classes do not use each-others private methods and fields if they will reside in different files.
Created attachment 180793 [details] Patch
Comment on attachment 180793 [details] Patch 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
Comment on attachment 180793 [details] Patch 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. fixed >> 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);) fixed >> Source/WebCore/inspector/front-end/DefaultTextEditor.js:458 >> + var firstVisibleLineNumber = this._mainPanel.findFirstVisibleLineNumber(visibleFrom); > > lineNumberAtOffset(visibleFrom) fixed >> 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. fixed >> Source/WebCore/inspector/front-end/DefaultTextEditor.js:590 >> + this.freeCachedElements(); > > You already have willHide, free cached elements from there. fixed >> 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. done >> 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 renamed
Created attachment 180802 [details] Patch
Comment on attachment 180802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180802&action=review > Source/WebCore/inspector/front-end/DefaultTextEditor.js:437 > + this._mainPanel.contextMenu(event.target, contextMenu); populateContextMenu > Source/WebCore/inspector/front-end/DefaultTextEditor.js:462 > + return this._mainPanel.getSelection(); selection() > Source/WebCore/inspector/front-end/DefaultTextEditor.js:-484 > - return this._mainPanel._lastSelection; lastSelection > Source/WebCore/inspector/front-end/DefaultTextEditor.js:556 > + this._boundSelectionChangeListener = this._mainPanel.handleSelectionChange.bind(this._mainPanel); You should register this listener within mainPanel's wasShown instead. > Source/WebCore/inspector/front-end/DefaultTextEditor.js:806 > + throw new Error("createNewChunk() is not implemented"); ... should be implemented by descendants
Comment on attachment 180802 [details] Patch Clearing flags on attachment: 180802 Committed r138527: <http://trac.webkit.org/changeset/138527>
All reviewed patches have been landed. Closing bug.