Changeset 247052 in webkit


Ignore:
Timestamp:
Jul 2, 2019 9:14:19 AM (5 years ago)
Author:
Matt Baker
Message:

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):

Location:
trunk/Source/WebInspectorUI
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebInspectorUI/ChangeLog

    r247043 r247052  
     12019-07-02  Matt Baker  <mattbaker@apple.com>
     2
     3        REGRESSION (r238563): Web Inspector: Selection is erratic when holding Up/Down on Network Table
     4        https://bugs.webkit.org/show_bug.cgi?id=193841
     5        <rdar://problem/47559124>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Selecting and revealing a row after reloading Table data, but before the
     10        layout that populates visible rows, could cause the Table to always be
     11        scrolled so that the revealed row is first.
     12
     13        This patch fixes `revealRow` by calculating the position of the row being
     14        revealed in the absence of its DOM element, so that the Table is only
     15        scrolled when necessary.
     16
     17        * UserInterface/Views/Table.js:
     18        (WI.Table.prototype.revealRow):
     19        (WI.Table.prototype._resizeColumnsAndFiller):
     20        Drive-by fix: use realOffsetWidth for consistency.
     21        (WI.Table.prototype._updateVisibleRows):
     22        (WI.Table.prototype._calculateOffsetHeight):
     23        (WI.Table.prototype._calculateScrollTop):
     24
    1252019-07-02  Devin Rousso  <drousso@apple.com>
    226
  • trunk/Source/WebInspectorUI/UserInterface/Views/Table.js

    r246560 r247052  
    366366            return;
    367367
    368         // Force our own scroll update because we may have scrolled.
    369         this._cachedScrollTop = NaN;
    370 
    371368        if (this._isRowVisible(rowIndex)) {
    372369            let row = this._cachedRows.get(rowIndex);
    373370            console.assert(row, "Visible rows should always be in the cache.");
    374             if (row)
     371            if (row) {
    375372                row.scrollIntoViewIfNeeded(false);
    376             this.needsLayout();
     373                this._cachedScrollTop = NaN;
     374                this.needsLayout();
     375            }
    377376        } else {
    378             this._scrollContainerElement.scrollTop = rowIndex * this._rowHeight;
    379             this.updateLayout();
     377            let rowPosition = rowIndex * this._rowHeight;
     378            let scrollableOffsetHeight = this._calculateOffsetHeight();
     379            let scrollTop = this._calculateScrollTop();
     380            let newScrollTop = NaN;
     381            if (rowPosition + this._rowHeight < scrollTop)
     382                newScrollTop = rowPosition;
     383            else if (rowPosition > scrollTop + scrollableOffsetHeight)
     384                newScrollTop = scrollTop + scrollableOffsetHeight - this._rowHeight;
     385
     386            if (!isNaN(newScrollTop)) {
     387                this._scrollContainerElement.scrollTop = newScrollTop;
     388                this.updateLayout();
     389            }
    380390        }
    381391    }
     
    856866    {
    857867        if (isNaN(this._cachedWidth) || !this._cachedWidth)
    858             this._cachedWidth = Math.floor(this._scrollContainerElement.getBoundingClientRect().width);
     868            this._cachedWidth = this._scrollContainerElement.realOffsetWidth;
    859869
    860870        // Not visible yet.
     
    10651075        let overflowPadding = updateOffsetThreshold * 3;
    10661076
    1067         if (isNaN(this._cachedScrollTop))
    1068             this._cachedScrollTop = this._scrollContainerElement.scrollTop;
    1069 
    1070         if (isNaN(this._cachedHeight) || !this._cachedHeight)
    1071             this._cachedHeight = Math.floor(this._scrollContainerElement.getBoundingClientRect().height);
    1072 
    1073         let scrollTop = this._cachedScrollTop;
    1074         let scrollableOffsetHeight = this._cachedHeight;
     1077        let scrollTop = this._calculateScrollTop();
     1078        let scrollableOffsetHeight = this._calculateOffsetHeight();
    10751079
    10761080        let visibleRowCount = Math.ceil((scrollableOffsetHeight + (overflowPadding * 2)) / rowHeight);
     
    14521456        return this.dataSource.tableRepresentedObjectForIndex(this, index);
    14531457    }
     1458
     1459    _calculateOffsetHeight()
     1460    {
     1461        if (isNaN(this._cachedHeight))
     1462            this._cachedHeight = this._scrollContainerElement.realOffsetHeight;
     1463        return this._cachedHeight;
     1464    }
     1465
     1466    _calculateScrollTop()
     1467    {
     1468        if (isNaN(this._cachedScrollTop))
     1469            this._cachedScrollTop = this._scrollContainerElement.scrollTop;
     1470        return this._cachedScrollTop;
     1471    }
    14541472};
    14551473
Note: See TracChangeset for help on using the changeset viewer.