Attachment 370982 Details for Bug 198228 – Patch

[patch] Patch

bug-198228-20190530150721.patch (text/plain), 8.19 KB, created by Matt Baker on 2019-05-30 15:07:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2019-05-30 15:07:22 PDT

Size: 8.19 KB

patch

obsolete

>Subversion Revision: 245902
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index da0997eba7504d228e0b1a20ae57385001f6fe2a..cc8b1ada4ae4538aa17db3a3ed4930aeff90beb2 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-05-30  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.treeElementRevealed):
>+        (WI.DebuggerSidebarPanel.prototype.createContentTreeOutline):
>+        (WI.DebuggerSidebarPanel.prototype._debuggerCallFramesDidChange):
>+
>+        * UserInterface/Views/DetailsSection.js:
>+        (WI.DetailsSection.prototype.get headerElement):
>+
>+        * UserInterface/Views/TreeElement.js:
>+        (WI.TreeElement.prototype.reveal):
>+
>+        * UserInterface/Views/TreeOutline.js:
>+        (WI.TreeOutline):
>+        (WI.TreeOutline.prototype.set treeElementRevealedHandler):
>+        (WI.TreeOutline.prototype.didRevealTreeElement):
>+
> 2019-05-28  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Timelines: spacing around pie chart is different between CPU and Memory
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js b/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
>index 8dc21756503bd603f8e71f9992bbb49f3414167f..3e962c3589250a331d0c12c197c215af95d55b05 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,37 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         return this._addScript(representedObject);
>     }
> 
>+    createContentTreeOutline({ignoreCookieRestoration, suppressFiltering} = {})
>+    {
>+        let detailsSections = [this._pauseReasonSection, this._callStackSection, this._breakpointsSection, this._scriptsSection];
>+
>+        function treeElementRevealed(treeElement)
>+        {
>+            let treeOutline = treeElement.treeOutline;
>+            console.assert(treeOutline);
>+            if (!treeOutline)
>+                return;
>+
>+            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 treeElementRect = treeElement.listItemElement.getBoundingClientRect();
>+            let topOffset = headerRect.bottom - treeElementRect.top;
>+            if (topOffset > 0)
>+                this.scrollElement.scrollBy(0, -topOffset);
>+        }
>+
>+        let treeOutline = super.createContentTreeOutline({ignoreCookieRestoration, suppressFiltering})
>+        treeOutline.treeElementRevealedHandler = treeElementRevealed;
>+        return treeOutline;
>+    }
>+
>     // Protected
> 
>     saveStateToCookie(cookie)
>@@ -862,6 +893,10 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
>         let target = event.data.target;
>         let treeElement = this._findThreadTreeElementForTarget(target);
>         treeElement.refresh();
>+
>+        let activeCallFrameTreeElement = this._callStackTreeOutline.selectedTreeElement;
>+        if (activeCallFrameTreeElement)
>+            activeCallFrameTreeElement.reveal();
>     }
> 
>     _debuggerActiveCallFrameDidChange()
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js b/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>index aa3b18f78968bb9c4eb3ad5f800874600a4210c6..7c27a5c4f2910024e55744694987fe2e1fb0512a 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DetailsSection.js
>@@ -85,6 +85,11 @@ WI.DetailsSection = class DetailsSection extends WI.Object
>         this._titleElement.textContent = title;
>     }
> 
>+    get headerElement()
>+    {
>+        return this._headerElement;
>+    }
>+
>     get titleElement()
>     {
>         return this._titleElement;
>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..fa2ce628143a95f8932e1c9522cfdcc8ee0ee0cd 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -117,6 +117,8 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         this._virtualizedTopSpacer = null;
>         this._virtualizedBottomSpacer = null;
> 
>+        this._treeElementRevealedHandler = null;
>+
>         this._childrenListNode.tabIndex = 0;
>         this._childrenListNode.addEventListener("keydown", this._treeKeyDown.bind(this), true);
>         this._childrenListNode.addEventListener("mousedown", this._handleMouseDown.bind(this));
>@@ -258,6 +260,12 @@ WI.TreeOutline = class TreeOutline extends WI.Object
> 
>     get selectable() { return this._selectable; }
> 
>+    set treeElementRevealedHandler(revealHandler)
>+    {
>+        console.assert(typeof revealHandler === "function");
>+        this._treeElementRevealedHandler = revealHandler;
>+    }
>+
>     appendChild(child)
>     {
>         console.assert(child);
>@@ -886,6 +894,12 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         return null;
>     }
> 
>+    didRevealTreeElement(treeElement)
>+    {
>+        if (this._treeElementRevealedHandler)
>+            this._treeElementRevealedHandler(treeElement);
>+    }
>+
>     // Protected
> 
>     canSelectTreeElement(treeElement)

Actions: View | Formatted Diff | Diff

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