Attachment 349830 Details for Bug 189638 – [PATCH] Proposed Fix

[patch] [PATCH] Proposed Fix

reveal-1.patch (text/plain), 4.71 KB, created by Joseph Pecoraro on 2018-09-14 16:55:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2018-09-14 16:55:01 PDT

Size: 4.71 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 2a3b3b39a62..60b7cd00e3d 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,33 @@
>+2018-09-14  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Source View doesn't scroll to show line when breakpoint is hit inside of <script> in HTML resource
>+        https://bugs.webkit.org/show_bug.cgi?id=189638
>+        <rdar://problem/43359278>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Pausing in Document resources on page load have special considerations where we
>+        may not receive their full content immediately, so we may synthesize inline script
>+        content for the resource until we have full content. Unfortunately, when we get
>+        full content and update the TextEditor we were losing the revealed position and
>+        other editor information. This change specifically targets those cases, by
>+        re-applying editor state when updating such a resource from partial to full content.
>+
>+        * UserInterface/Views/SourceCodeTextEditor.js:
>+        (WI.SourceCodeTextEditor.prototype._contentAvailable):
>+        Signal to the editor that when updating the content it should carry
>+        over editor state. Only do this if we had partial content before.
>+
>+        * UserInterface/Views/TextEditor.js:
>+        (WI.TextEditor):
>+        (WI.TextEditor.prototype.set repeatReveal):
>+        Provide a way to signal we should restore state when update the content.
>+
>+        (WI.TextEditor.set string.update):
>+        (WI.TextEditor.prototype.set string):
>+        When repeatReveal was enabled, restore editor state such as the
>+        revealed location and text selection.
>+
> 2018-09-13  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Opening inspector with a selected element might immediately scroll that element off screen in the DOM Tree outline
>diff --git a/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js b/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js
>index 12e5de2d511..93b3b9a3554 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js
>@@ -547,7 +547,13 @@ WI.SourceCodeTextEditor = class SourceCodeTextEditor extends WI.TextEditor
>         this._fullContentPopulated = true;
>         this._invalidLineNumbers = {};
> 
>+        // If we had partial content (such as inline script content) before we had full content, we
>+        // will want to re-restore the revealed position now that we are populating with full content.
>+        this.repeatReveal = !!this.string;
>+
>         this._populateWithContent(content);
>+
>+        this.repeatReveal = false;
>     }
> 
>     _breakpointStatusDidChange(event)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TextEditor.js b/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>index ad00dde6d09..25ea4a185d6 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TextEditor.js
>@@ -94,6 +94,7 @@ WI.TextEditor = class TextEditor extends WI.View
>         this._formattingPromise = null;
>         this._formatterSourceMap = null;
>         this._deferReveal = false;
>+        this._repeatReveal = false;
> 
>         this._delegate = delegate || null;
>     }
>@@ -112,6 +113,8 @@ WI.TextEditor = class TextEditor extends WI.View
> 
>     set string(newString)
>     {
>+        let previousSelectedTextRange = this._repeatReveal ? this.selectedTextRange : null;
>+
>         function update()
>         {
>             // Clear any styles that may have been set on the empty line before content loaded.
>@@ -147,8 +150,13 @@ WI.TextEditor = class TextEditor extends WI.View
>             for (var lineNumber in this._breakpoints)
>                 this._setBreakpointStylesOnLine(lineNumber);
> 
>-            // Try revealing the pending line now that we might have content with enough lines.
>+            // Try revealing the pending line, or previous position, now that we might have new content.
>             this._revealPendingPositionIfPossible();
>+            if (previousSelectedTextRange) {
>+                this.selectedTextRange = previousSelectedTextRange;
>+                let position = this._codeMirrorPositionFromTextRange(previousSelectedTextRange);
>+                this._scrollIntoViewCentered(position.start);
>+            }
>         }
> 
>         this._ignoreCodeMirrorContentDidChangeEvent++;
>@@ -292,6 +300,11 @@ WI.TextEditor = class TextEditor extends WI.View
>         this._deferReveal = defer;
>     }
> 
>+    set repeatReveal(repeat)
>+    {
>+        this._repeatReveal = repeat;
>+    }
>+
>     performSearch(query)
>     {
>         if (this._searchQuery === query)

Actions: View | Formatted Diff | Diff

Attachments on bug 189638: 349829 | 349830