Attachment 371166 Details for Bug 198228 – Patch for landing

[patch] Patch for landing

bug-198228-20190602153947.patch (text/plain), 10.37 KB, created by Matt Baker on 2019-06-02 15:39:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2019-06-02 15:39:47 PDT

Size: 10.37 KB

patch

obsolete

>Subversion Revision: 246023
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 50ea47c82a6a5a0912b6cc19a4fe1a07248b8669..3a5c61eea00b6f3793539c23190e88445af8fd0d 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 Devin Rousso.
>+
>+        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/Base/Utilities.js:
>+
>+        * 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:
>+
> 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/Base/Utilities.js b/Source/WebInspectorUI/UserInterface/Base/Utilities.js
>index ebd71e042806e3eec80dfb96f6d4b0935820289d..b0b2446d490709406866fb827d7ee27706a9fa13 100644
>--- a/Source/WebInspectorUI/UserInterface/Base/Utilities.js
>+++ b/Source/WebInspectorUI/UserInterface/Base/Utilities.js
>@@ -388,6 +388,14 @@ Object.defineProperty(Element.prototype, "totalOffsetTop",
>     }
> });
> 
>+Object.defineProperty(Element.prototype, "totalOffsetBottom",
>+{
>+    get()
>+    {
>+        return this.getBoundingClientRect().bottom;
>+    }
>+});
>+
> Object.defineProperty(Element.prototype, "removeChildren",
> {
>     value()
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>index 8dc21756503bd603f8e71f9992bbb49f3414167f..9bb6a68485e8fbcddd5e560742e5d761075d0ede 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,29 @@ 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.element;
>+            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 offset = detailsSection.headerElement.totalOffsetBottom - treeElement.listItemElement.totalOffsetTop;
>+            if (offset > 0)
>+                this.scrollElement.scrollBy(0, -offset);
>+        });
>+
>+        return treeOutline;
>+    }
>+
>     // Protected
> 
>     saveStateToCookie(cookie)
>@@ -862,6 +885,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..8e9dd14bf1cc53fd8e2a2204a140e4dca7a0c766 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
>@@ -474,6 +474,29 @@ 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.element;
>+            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 offset = detailsSection.headerElement.totalOffsetBottom - treeElement.listItemElement.totalOffsetTop;
>+            if (offset > 0)
>+                this.scrollElement.scrollBy(0, -offset);
>+        });
>+
>+        return treeOutline;
>+    }
>+
>     resetFilter()
>     {
>         this._resourceTypeScopeBar.resetToDefault();
>@@ -1728,6 +1751,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..f6ae0469dc0b193daa9b22cee75f5efc3377716b 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.dispatchEventToListeners(WI.TreeOutline.Event.ElementRevealed, {element: this});
>     }
> 
>     revealed(ignoreHidden)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>index 66ecd52bb8f3391cc3732f08001bed6c4f42ec41..970982d3ce9bb1deee4b31312747cbbe8f52e2b7 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -1159,6 +1159,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