[webkit-changes] [WebKit/WebKit] ba617a: Web Inspector: REGRESSION(?): Storage: arrow key m...

[Devin Rousso]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ba617a2832c014b8ccfef7fbd21db69a0fcdc47d
      https://github.com/WebKit/WebKit/commit/ba617a2832c014b8ccfef7fbd21db69a0fcdc47d
  Author: Devin Rousso <hi at devinrousso.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js

  Log Message:
  -----------
  Web Inspector: REGRESSION(?): Storage: arrow key movement sometimes skips items
https://bugs.webkit.org/show_bug.cgi?id=257608

Reviewed by Patrick Angle.

`WI.DataGrid` has two different ways of managing the state of its children:
- `_children` represents the "original" order of children before any sorting happens.
- `_rows` contains the visual ordering of children after sorting and whatnot.

The logic that calculates the `previousSibling`/`nextSibling` for each `WI.DataGridNode` happens in two different places:
1. `WI.DataGridNode.prototype._recalculateSiblings`, which is called by `WI.DataGrid.prototype.insertChild`.
2. `WI.DataGrid.prototype._sortNodesCallback`, specifically after sorting has finished.

(1) uses `_children` and (2) uses `_rows`, which can lead to issues if the caller doesn't sort the `WI.DataGrid` *after* adding any new `WI.DataGridNode` as in that case the item before it in `_children` might not actually be the item before it in `_rows`.

Imagine we have a `WI.DataGrid` with two children added in reverse alphabetical order but sorted alphabetically:

    _children:
        b { previous: a, next: _ }
        a { previous: _, next: b }

    _rows:
        a { previous: _, next: b }
        b { previous: a, next: _ }

If we now added another `WI.DataGridNode` for "z" *without* sorting, we'd end up with this:

    _children:
        b { previous: a, next: _ }
        a { previous: _, next: z }
        z { previous: a, next: _ }

    _rows:
        a { previous: _, next: z }
        b { previous: a, next: _ }
        z { previous: a, next: _ }

Which would cause the user to skip over "b" when pressing the down arrow key if "a" is selected.

I am somewhat doubtful that this is an issue elsewhere in Web Inspector as `WI.PlaceholderDataGridNode` is not used anywhere else (and `WI.DataGrid` is sorta unofficially softly deprecated in favor of `WI.Table`).

* Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js:
(WI.DOMStorageContentView.prototype._populate):
Always sort *after* adding `WI.DataGridNode`, not before.

Canonical link: https://commits.webkit.org/264823@main