Attachment 373292 Details for Bug 193841 – Patch

[patch] Patch

bug-193841-20190701180229.patch (text/plain), 4.88 KB, created by Matt Baker on 2019-07-01 18:02:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2019-07-01 18:02:31 PDT

Size: 4.88 KB

patch

obsolete

>Subversion Revision: 247016
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 70ac1bbfdbf6b62cfc548b107a8a577ceda082ec..da7d401491dbf10536fcfb219b001694e9369b05 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-07-01  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 NOBODY (OOPS!).
>+
>+        Selecting and revealing a row after reload 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._ensureOffsetHeight):
>+        (WI.Table.prototype._ensureScrollTop):
>+
> 2019-06-29  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: Remove trailing white-space
>diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.js b/Source/WebInspectorUI/UserInterface/Views/Table.js
>index e44a7c26182ec6c36f6af4919aa315cc04814d6c..f6370cab0deb434abaa3ca0aff65e04d77950e19 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/Table.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/Table.js
>@@ -365,9 +365,6 @@ 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.");
>@@ -375,8 +372,19 @@ WI.Table = class Table extends WI.View
>                 row.scrollIntoViewIfNeeded(false);
>             this.needsLayout();
>         } else {
>-            this._scrollContainerElement.scrollTop = rowIndex * this._rowHeight;
>-            this.updateLayout();
>+            let rowPosition = rowIndex * this._rowHeight;
>+            let scrollableOffsetHeight = this._ensureOffsetHeight();
>+            let scrollTop = this._ensureScrollTop();
>+            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 +863,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 +1072,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._ensureScrollTop();
>+        let scrollableOffsetHeight = this._ensureOffsetHeight();
> 
>         let visibleRowCount = Math.ceil((scrollableOffsetHeight + (overflowPadding * 2)) / rowHeight);
>         let currentTopMargin = this._topSpacerHeight;
>@@ -1451,6 +1453,20 @@ WI.Table = class Table extends WI.View
>     {
>         return this.dataSource.tableRepresentedObjectForIndex(this, index);
>     }
>+
>+    _ensureOffsetHeight()
>+    {
>+        if (isNaN(this._cachedHeight))
>+            this._cachedHeight = this._scrollContainerElement.realOffsetHeight;
>+        return this._cachedHeight;
>+    }
>+
>+    _ensureScrollTop()
>+    {
>+        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