REGRESSION (r238563): Web Inspector: Selection is erratic when holding Up/Down on...
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Jul 2019 16:14:19 +0000 (16:14 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Jul 2019 16:14:19 +0000 (16:14 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247052 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Views/Table.js

index e07aafd..ae2f193 100644 (file)
@@ -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
index e44a7c2..7bd7d6b 100644 (file)
@@ -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 = {