Attachment 356619 Details for Bug 192388 – Patch

[patch] Patch

bug-192388-20181205105552.patch (text/plain), 6.62 KB, created by Matt Baker on 2018-12-05 10:56:00 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2018-12-05 10:56:00 PST

Size: 6.62 KB

patch

obsolete

>Subversion Revision: 238877
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 9271caa8a2edab6b000f0e46c3c0d9f4824a98a0..d4fcfd4064c505f77f08d5a966b891ebe516ba07 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-12-05  Matt Baker  <mattbaker@apple.com>
>+
>+        Web Inspector: Table selection becomes corrupted when deleting selected cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=192388
>+        <rdar://problem/46472364>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UserInterface/Controllers/SelectionController.js:
>+        (WI.SelectionController.prototype.didRemoveItems):
>+        (WI.SelectionController.prototype.didRemoveItem): Deleted.
>+        Replace `didRemoveItem` with a method taking an IndexSet. Calling the
>+        single-index version while iterating over multiple rows in ascending
>+        order is unsafe, a detail best left to the SelectionController.
>+
>+        * UserInterface/Views/Table.js:
>+        (WI.Table.prototype.removeRow):
>+        (WI.Table.prototype._removeRows):
>+
>+        * UserInterface/Views/TreeOutline.js:
>+        (WI.TreeOutline.prototype.removeChildAtIndex):
>+
> 2018-12-04  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: REGRESSION(r238602): Elements: changing selection no longer highlights the selected node
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>index ea9e3bff5a4fc2e91c57a735ea858862498ae308..12eab8b7713067bc226f20cae2b9d600c9449b56 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>@@ -210,14 +210,25 @@ WI.SelectionController = class SelectionController extends WI.Object
>         }
>     }
> 
>-    didRemoveItem(index)
>+    didRemoveItems(indexes)
>     {
>-        if (this.hasSelectedItem(index))
>-            this.deselectItem(index);
>+        console.assert(indexes instanceof WI.IndexSet);
> 
>-        while (index = this._selectedIndexes.indexGreaterThan(index)) {
>-            this._selectedIndexes.delete(index);
>-            this._selectedIndexes.add(index - 1);
>+        if (!this._selectedIndexes.size)
>+            return;
>+
>+        if (this._selectedIndexes.lastIndex < indexes.firstIndex)
>+            return;
>+
>+        for (let index = indexes.lastIndex; index >= indexes.firstIndex; index = indexes.indexLessThan(index)) {
>+            if (this.hasSelectedItem(index))
>+                this.deselectItem(index);
>+
>+            let nextIndex = index;
>+            while (nextIndex = this._selectedIndexes.indexGreaterThan(nextIndex)) {
>+                this._selectedIndexes.delete(nextIndex);
>+                this._selectedIndexes.add(nextIndex - 1);
>+            }
>         }
>     }
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.js b/Source/WebInspectorUI/UserInterface/Views/Table.js
>index acd0cf4300d385b50c9103f081ac4840aed6c813..2bc6f34b602bd2425d889f790bec563c99e5f63d 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/Table.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/Table.js
>@@ -338,8 +338,8 @@ WI.Table = class Table extends WI.View
>         if (this.isRowSelected(rowIndex))
>             this.deselectRow(rowIndex);
> 
>-        this._removeRows(new WI.IndexSet([rowIndex]));
>-        this._selectionController.didRemoveItem(rowIndex);
>+        let rowIndexes = new WI.IndexSet([rowIndex]);
>+        this._removeRows(rowIndexes);
>     }
> 
>     removeSelectedRows()
>@@ -1393,6 +1393,8 @@ WI.Table = class Table extends WI.View
>         this._cachedNumberOfRows -= removed;
>         console.assert(this._cachedNumberOfRows >= 0);
> 
>+        this._selectionController.didRemoveItems(rowIndexes);
>+
>         if (this._delegate.tableDidRemoveRows) {
>             this._delegate.tableDidRemoveRows(this, Array.from(rowIndexes));
>             console.assert(this._cachedNumberOfRows === this._dataSource.tableNumberOfRows(this), "Table data source should update after removing rows.");
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>index 72f2cfdaf17e6266b475ae9d417054d30edb33dc..5ae924edb5a6ab99d75729c440ec2f057a43626d 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -339,7 +339,7 @@ WI.TreeOutline = class TreeOutline extends WI.Object
>         if (treeOutline) {
>             treeOutline._forgetTreeElement(child);
>             treeOutline._forgetChildrenRecursive(child);
>-            treeOutline._selectionController.didRemoveItem(childIndex);
>+            treeOutline._selectionController.didRemoveItems(new WI.IndexSet([childIndex]));
>         }
> 
>         child._detach();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b9c902bcf41b5f730cc441dd2d7f658b10c4f6e1..f3d705ee5bf8534574e90cf97945cc68df9046d3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-12-05  Matt Baker  <mattbaker@apple.com>
>+
>+        Web Inspector: Table selection becomes corrupted when deleting selected cookies
>+        https://bugs.webkit.org/show_bug.cgi?id=192388
>+        <rdar://problem/46472364>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/table/table-remove-rows.html:
>+
> 2018-11-30  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         Don't report resource timing to parent frame for history items
>diff --git a/LayoutTests/inspector/table/table-remove-rows.html b/LayoutTests/inspector/table/table-remove-rows.html
>index c4615033f822c03de16e33b7196691f26a460bef..358e0491bd5a480ed5b872a0dd57d8534039d192 100644
>--- a/LayoutTests/inspector/table/table-remove-rows.html
>+++ b/LayoutTests/inspector/table/table-remove-rows.html
>@@ -134,6 +134,24 @@ function test()
>         }
>     });
> 
>+    suite.addTestCase({
>+        name: "Table.RemoveRow.PrecedingSelected",
>+        description: "Remove a row preceding the selection, causing the selection to shift up.",
>+        test() {
>+            let testDelegate = new RemoveRowTestDelegate;
>+            let table = InspectorTest.createTableWithDelegate(testDelegate, numberOfRows);
>+            table.allowsMultipleSelection = true;
>+
>+            table.selectRow(1);
>+            table.selectRow(3, true);
>+            testDelegate.triggerRemoveRow(table, 0);
>+
>+            InspectorTest.expectShallowEqual(table.selectedRows, [0, 2], "Selected row indexes should be adjusted.");
>+
>+            return true;
>+        }
>+    });
>+
>     function addTestCase({name, description, rowIndexes}) {
>         suite.addTestCase({
>             name, description,

Actions: View | Formatted Diff | Diff

Attachments on bug 192388: 356619 | 356636 | 356637 | 356642 | 356687 | 356692 | 356772 | 357161 | 357162 | 357168 | 357169 | 357175 | 357181 | 357187 | 357203