Attachment 373326 Details for Bug 193841 – Patch for landing

[patch] Patch for landing

bug-193841-20190702085546.patch (text/plain), 5.04 KB, created by Matt Baker on 2019-07-02 08:55:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2019-07-02 08:55:47 PDT

Size: 5.04 KB

patch

obsolete

>Subversion Revision: 247050
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index e07aafd029c1450af5e4c0ecfd863aac482be69d..ae2f193109169325b94207a757d109a6650adddf 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-07-02  Matt Baker  <mattbaker@apple.com>
>+
>+        REGRESSION (r238563): Web Inspector: Selection is erratic when holding Up/Down on Network Table
>+        https://bugs.webkit.org/show_bug.cgi?id=193841
>+        <rdar://problem/47559124>
>+
>+        Reviewed by Devin Rousso.
>+
>+        Selecting and revealing a row after reloading Table data, but before the
>+        layout that populates visible rows, could cause the Table to always be
>+        scrolled so that the revealed row is first.
>+
>+        This patch fixes `revealRow` by calculating the position of the row being
>+        revealed in the absence of its DOM element, so that the Table is only
>+        scrolled when necessary.
>+
>+        * UserInterface/Views/Table.js:
>+        (WI.Table.prototype.revealRow):
>+        (WI.Table.prototype._resizeColumnsAndFiller):
>+        Drive-by fix: use realOffsetWidth for consistency.
>+        (WI.Table.prototype._updateVisibleRows):
>+        (WI.Table.prototype._calculateOffsetHeight):
>+        (WI.Table.prototype._calculateScrollTop):
>+
> 2019-07-02  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Debug: "Reset Web Inspector" should also clear the saved window size and attachment side
>diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.js b/Source/WebInspectorUI/UserInterface/Views/Table.js
>index e44a7c26182ec6c36f6af4919aa315cc04814d6c..7bd7d6bbcca184167865359910b1c56239939d4c 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/Table.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/Table.js
>@@ -365,18 +365,28 @@ WI.Table = class Table extends WI.View
>         if (rowIndex < 0 || rowIndex >= this.numberOfRows)
>             return;
> 
>-        // Force our own scroll update because we may have scrolled.
>-        this._cachedScrollTop = NaN;
>-
>         if (this._isRowVisible(rowIndex)) {
>             let row = this._cachedRows.get(rowIndex);
>             console.assert(row, "Visible rows should always be in the cache.");
>-            if (row)
>+            if (row) {
>                 row.scrollIntoViewIfNeeded(false);
>-            this.needsLayout();
>+                this._cachedScrollTop = NaN;
>+                this.needsLayout();
>+            }
>         } else {
>-            this._scrollContainerElement.scrollTop = rowIndex * this._rowHeight;
>-            this.updateLayout();
>+            let rowPosition = rowIndex * this._rowHeight;
>+            let scrollableOffsetHeight = this._calculateOffsetHeight();
>+            let scrollTop = this._calculateScrollTop();
>+            let newScrollTop = NaN;
>+            if (rowPosition + this._rowHeight < scrollTop)
>+                newScrollTop = rowPosition;
>+            else if (rowPosition > scrollTop + scrollableOffsetHeight)
>+                newScrollTop = scrollTop + scrollableOffsetHeight - this._rowHeight;
>+
>+            if (!isNaN(newScrollTop)) {
>+                this._scrollContainerElement.scrollTop = newScrollTop;
>+                this.updateLayout();
>+            }
>         }
>     }
> 
>@@ -855,7 +865,7 @@ WI.Table = class Table extends WI.View
>     _resizeColumnsAndFiller()
>     {
>         if (isNaN(this._cachedWidth) || !this._cachedWidth)
>-            this._cachedWidth = Math.floor(this._scrollContainerElement.getBoundingClientRect().width);
>+            this._cachedWidth = this._scrollContainerElement.realOffsetWidth;
> 
>         // Not visible yet.
>         if (!this._cachedWidth)
>@@ -1064,14 +1074,8 @@ WI.Table = class Table extends WI.View
>         let updateOffsetThreshold = rowHeight * 10;
>         let overflowPadding = updateOffsetThreshold * 3;
> 
>-        if (isNaN(this._cachedScrollTop))
>-            this._cachedScrollTop = this._scrollContainerElement.scrollTop;
>-
>-        if (isNaN(this._cachedHeight) || !this._cachedHeight)
>-            this._cachedHeight = Math.floor(this._scrollContainerElement.getBoundingClientRect().height);
>-
>-        let scrollTop = this._cachedScrollTop;
>-        let scrollableOffsetHeight = this._cachedHeight;
>+        let scrollTop = this._calculateScrollTop();
>+        let scrollableOffsetHeight = this._calculateOffsetHeight();
> 
>         let visibleRowCount = Math.ceil((scrollableOffsetHeight + (overflowPadding * 2)) / rowHeight);
>         let currentTopMargin = this._topSpacerHeight;
>@@ -1451,6 +1455,20 @@ WI.Table = class Table extends WI.View
>     {
>         return this.dataSource.tableRepresentedObjectForIndex(this, index);
>     }
>+
>+    _calculateOffsetHeight()
>+    {
>+        if (isNaN(this._cachedHeight))
>+            this._cachedHeight = this._scrollContainerElement.realOffsetHeight;
>+        return this._cachedHeight;
>+    }
>+
>+    _calculateScrollTop()
>+    {
>+        if (isNaN(this._cachedScrollTop))
>+            this._cachedScrollTop = this._scrollContainerElement.scrollTop;
>+        return this._cachedScrollTop;
>+    }
> };
> 
> WI.Table.SortOrder = {

Actions: View | Formatted Diff | Diff

Attachments on bug 193841: 373091 | 373185 | 373280 | 373285 | 373291 | 373292 | 373326