Attachment 371162 Details for Bug 198228 – Patch

[patch] Patch

bug-198228-20190602143631.patch (text/plain), 10.18 KB, created by Matt Baker on 2019-06-02 14:36:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2019-06-02 14:36:32 PDT

Size: 10.18 KB

patch

obsolete

>Subversion Revision: 246023
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 50ea47c82a6a5a0912b6cc19a4fe1a07248b8669..d729b56b39e1d3aa3d9da990d2433808053fa7cf 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,39 @@
>+2019-06-02  Matt Baker  <mattbaker@apple.com>
>+
>+        Web Inspector: Debugger: sidebar should always reveal active call frame when hitting a breakpoint
>+        https://bugs.webkit.org/show_bug.cgi?id=198228
>+        <rdar://problem/46719447>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Reveal the active call frame TreeElement when call frames change. Refreshing
>+        the current target's ThreadTreeElement children is insufficient, since
>+        the sidebar panel content may have been scrolled.
>+
>+        This patch also introduces a workaround to prevent the DetailsSection header
>+        element, which has sticky positioning, from covering a revealed TreeElement.
>+        This can be the case when the TreeElement being revealed is at the topmost edge
>+        of the scrolled content element.
>+
>+        * UserInterface/Views/DebuggerSidebarPanel.js:
>+        (WI.DebuggerSidebarPanel.prototype.createContentTreeOutline):
>+        (WI.DebuggerSidebarPanel.prototype._debuggerCallFramesDidChange):
>+
>+        * UserInterface/Views/DetailsSection.js:
>+        (WI.DetailsSection.prototype.get element):
>+        (WI.DetailsSection.prototype.get headerElement):
>+        (WI.DetailsSection.prototype.get identifier):
>+
>+        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
>+        (WI.SourcesNavigationSidebarPanel.prototype.createContentTreeOutline):
>+        (WI.SourcesNavigationSidebarPanel.prototype._handleDebuggerCallFramesDidChange):
>+
>+        * UserInterface/Views/TreeElement.js:
>+        (WI.TreeElement.prototype.reveal):
>+
>+        * UserInterface/Views/TreeOutline.js:
>+        (WI.TreeOutline.prototype.didRevealTreeElement):
>+
> 2019-05-31  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>index 8dc21756503bd603f8e71f9992bbb49f3414167f..7e4d47b7a45784cfdb69453c629a627d3b49eb67 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>@@ -161,8 +161,8 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         breakpointNavigationBar.addNavigationItem(this._createBreakpointButton);
> 
>         let breakpointsGroup = new WI.DetailsSectionGroup([breakpointsRow]);
>-        let breakpointsSection = new WI.DetailsSection("breakpoints", WI.UIString("Breakpoints"), [breakpointsGroup], breakpointNavigationBarWrapper);
>-        this.contentView.element.appendChild(breakpointsSection.element);
>+        this._breakpointsSection = new WI.DetailsSection("breakpoints", WI.UIString("Breakpoints"), [breakpointsGroup], breakpointNavigationBarWrapper);
>+        this.contentView.element.appendChild(this._breakpointsSection.element);
> 
>         this._breakpointsContentTreeOutline.addEventListener(WI.TreeOutline.Event.ElementAdded, this._handleBreakpointElementAddedOrRemoved, this);
>         this._breakpointsContentTreeOutline.addEventListener(WI.TreeOutline.Event.ElementRemoved, this._handleBreakpointElementAddedOrRemoved, this);
>@@ -329,6 +329,30 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         return this._addScript(representedObject);
>     }
> 
>+    createContentTreeOutline(options = {})
>+    {
>+        let treeOutline = super.createContentTreeOutline(options)
>+
>+        treeOutline.addEventListener(WI.TreeOutline.Event.ElementRevealed, (event) => {
>+            let treeElement = event.data.treeElement;
>+            let detailsSections = [this._pauseReasonSection, this._callStackSection, this._breakpointsSection, this._scriptsSection];
>+            let detailsSection = detailsSections.find((detailsSection) => detailsSection.element.contains(treeElement.listItemElement));
>+            if (!detailsSection)
>+                return;
>+
>+            // Revealing a TreeElement at the scroll container's topmost edge with
>+            // scrollIntoViewIfNeeded may result in the element being covered by the
>+            // DetailsSection header, which uses sticky positioning. Detect this case,
>+            // and adjust the sidebar content's scroll position to compensate.
>+            let headerRect = detailsSection.headerElement.getBoundingClientRect();
>+            let topOffset = headerRect.bottom - treeElement.listItemElement.totalOffsetTop;
>+            if (topOffset > 0)
>+                this.scrollElement.scrollBy(0, -topOffset);
>+        });
>+
>+        return treeOutline;
>+    }
>+
>     // Protected
> 
>     saveStateToCookie(cookie)
>@@ -862,6 +886,10 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         let target = event.data.target;
>         let treeElement = this._findThreadTreeElementForTarget(target);
>         treeElement.refresh();
>+
>+        let activeCallFrameTreeElement = this._callStackTreeOutline.findTreeElement(WI.debuggerManager.activeCallFrame);
>+        if (activeCallFrameTreeElement)
>+            activeCallFrameTreeElement.reveal();
>     }
> 
>     _debuggerActiveCallFrameDidChange()
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js b/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>index aa3b18f78968bb9c4eb3ad5f800874600a4210c6..0a9022a079a27cf369e344bf1a3bd3a1aa923835 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>@@ -65,15 +65,9 @@ WI.DetailsSection = class DetailsSection extends WI.Object
> 
>     // Public
> 
>-    get element()
>-    {
>-        return this._element;
>-    }
>-
>-    get identifier()
>-    {
>-        return this._identifier;
>-    }
>+    get element() { return this._element; }
>+    get headerElement() { return this._headerElement; }
>+    get identifier() { return this._identifier; }
> 
>     get title()
>     {
>diff --git a/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
>index c5a1bfe14eaeb5877adb5e47b2f657884a8ae6fc..15dc7e88ac8dad764008a1e86cdc832fe0c20614 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
>@@ -474,6 +474,30 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
> 
>     // Protected
> 
>+    createContentTreeOutline(options = {})
>+    {
>+        let treeOutline = super.createContentTreeOutline(options)
>+
>+        treeOutline.addEventListener(WI.TreeOutline.Event.ElementRevealed, (event) => {
>+            let treeElement = event.data.treeElement;
>+            let detailsSections = [this._pauseReasonSection, this._callStackSection, this._breakpointsSection];
>+            let detailsSection = detailsSections.find((detailsSection) => detailsSection.element.contains(treeElement.listItemElement));
>+            if (!detailsSection)
>+                return;
>+
>+            // Revealing a TreeElement at the scroll container's topmost edge with
>+            // scrollIntoViewIfNeeded may result in the element being covered by the
>+            // DetailsSection header, which uses sticky positioning. Detect this case,
>+            // and adjust the sidebar content's scroll position to compensate.
>+            let headerRect = detailsSection.headerElement.getBoundingClientRect();
>+            let topOffset = headerRect.bottom - treeElement.listItemElement.totalOffsetTop;
>+            if (topOffset > 0)
>+                this.scrollElement.scrollBy(0, -topOffset);
>+        });
>+
>+        return treeOutline;
>+    }
>+
>     resetFilter()
>     {
>         this._resourceTypeScopeBar.resetToDefault();
>@@ -1728,6 +1752,10 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
>         console.assert(treeElement);
>         if (treeElement)
>             treeElement.refresh();
>+
>+        let activeCallFrameTreeElement = this._callStackTreeOutline.findTreeElement(WI.debuggerManager.activeCallFrame);
>+        if (activeCallFrameTreeElement)
>+            activeCallFrameTreeElement.reveal();
>     }
> 
>     _handleDebuggerActiveCallFrameDidChange(event)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeElement.js b/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>index 63c27b8f1608094a36477ee4fa042f75669e1661..12ea5a6428775fdad4a44ca7b93f3beb869d8bfb 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeElement.js
>@@ -488,6 +488,9 @@ WI.TreeElement = class TreeElement extends WI.Object
> 
>         if (this.onreveal)
>             this.onreveal(this);
>+
>+        if (this.treeOutline)
>+            this.treeOutline.didRevealTreeElement(this);
>     }
> 
>     revealed(ignoreHidden)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>index 66ecd52bb8f3391cc3732f08001bed6c4f42ec41..485155ba0cd66da7d54a589d24e9f356bb47da51 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -886,6 +886,13 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         return null;
>     }
> 
>+    didRevealTreeElement(treeElement)
>+    {
>+        console.assert(treeElement.treeOutline === this);
>+
>+        this.dispatchEventToListeners(TreeOutline.Event.ElementRevealed, {treeElement});
>+    }
>+
>     // Protected
> 
>     canSelectTreeElement(treeElement)
>@@ -1159,6 +1166,7 @@ WI.TreeOutline.Event = {
>     ElementAdded: Symbol("element-added"),
>     ElementDidChange: Symbol("element-did-change"),
>     ElementRemoved: Symbol("element-removed"),
>+    ElementRevealed: Symbol("element-revealed"),
>     ElementClicked: Symbol("element-clicked"),
>     ElementDisclosureDidChanged: Symbol("element-disclosure-did-change"),
>     ElementVisibilityDidChange: Symbol("element-visbility-did-change"),

Actions: View | Formatted Diff | Diff

Attachments on bug 198228: 370582 | 370982 | 371162 | 371165 | 371166