WebKit Bugzilla
Attachment 350154 Details for
Bug 189705
: Web Inspector: Table should support multiple selection and Cmd-click behavior
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-189705-20180919154516.patch (text/plain), 44.67 KB, created by
Matt Baker
on 2018-09-19 15:45:27 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Matt Baker
Created:
2018-09-19 15:45:27 PDT
Size:
44.67 KB
patch
obsolete
>Subversion Revision: 236228 >diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog >index 0328d8118bf8b7d7ff1168830c7bfa9ac95550ac..92894bf20e7d7791ed51e70ca584ee08c77326e7 100644 >--- a/Source/WebInspectorUI/ChangeLog >+++ b/Source/WebInspectorUI/ChangeLog >@@ -1,3 +1,80 @@ >+2018-09-19 Matt Baker <mattbaker@apple.com> >+ >+ Web Inspector: Table should support multiple selection and Cmd-click behavior >+ https://bugs.webkit.org/show_bug.cgi?id=189705 >+ <rdar://problem/44571170> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add multiple row selection to Table, with new methods for programmatic >+ selection (deselectRow, deselectAll), and Command-click support for >+ selecting/deselecting Table rows. >+ >+ * UserInterface/Base/IndexSet.js: Added. >+ (WI.IndexSet): >+ (WI.IndexSet.prototype.get size): >+ (WI.IndexSet.prototype.get firstIndex): >+ (WI.IndexSet.prototype.get lastIndex): >+ (WI.IndexSet.prototype.add): >+ (WI.IndexSet.prototype.delete): >+ (WI.IndexSet.prototype.has): >+ (WI.IndexSet.prototype.clear): >+ (WI.IndexSet.prototype.indexGreaterThan): >+ (WI.IndexSet.prototype.indexLessThan): >+ (WI.IndexSet.prototype.Symbol.iterator): >+ (WI.IndexSet.prototype._indexClosestTo): >+ (WI.IndexSet.prototype._validateIndex): >+ Helper container for managing an ordered sequence of unique positive >+ integers, with set semantics, backed by a sorted array. Used by Table, >+ and eventually by TreeOutline. >+ >+ * UserInterface/Main.html: >+ * UserInterface/Test.html: >+ * UserInterface/Test/Test.js: >+ New files and stubs to make Table layout tests possible. >+ >+ * UserInterface/Views/DarkMode.css: >+ (@media (prefers-dark-interface)): >+ (.table > .data-container > .data-list > li.selected + li.selected): >+ >+ * UserInterface/Views/NetworkTableContentView.js: >+ (WI.NetworkTableContentView.prototype.reset): >+ (WI.NetworkTableContentView.prototype.showRepresentedObject): >+ (WI.NetworkTableContentView.prototype.networkResourceDetailViewClose): >+ (WI.NetworkTableContentView.prototype.tableSelectionDidChange): >+ (WI.NetworkTableContentView.prototype._restoreSelectedRow): >+ (WI.NetworkTableContentView.prototype.tableSelectedRowChanged): Deleted. >+ Replace uses of `clearSelectedRow` with `deselectAll`, and updated >+ selection changed delegate. >+ >+ * UserInterface/Views/Table.css: >+ (.table > .data-container > .data-list > li): >+ (.table > .data-container > .data-list > li.selected + li.selected): >+ Style change to create a 1px divider between adjacent selected rows >+ to prevent selected row background colors from running together. >+ >+ * UserInterface/Views/Table.js: >+ (WI.Table): >+ (WI.Table.prototype.get selectedRows): >+ (WI.Table.prototype.get allowsMultipleSelection): >+ (WI.Table.prototype.set allowsMultipleSelection): >+ (WI.Table.prototype.reloadData): >+ (WI.Table.prototype.selectRow): >+ (WI.Table.prototype.deselectRow): >+ (WI.Table.prototype.deselectAll): >+ (WI.Table.prototype._getOrCreateRow): >+ (WI.Table.prototype._handleMouseDown): >+ (WI.Table.prototype._deselectAllAndSelect): >+ (WI.Table.prototype._isRowSelected): >+ (WI.Table.prototype._notifySelectionDidChange): >+ (WI.Table.prototype.clearSelectedRow): Deleted. >+ Table now tracks selected rows using an IndexSet. selectRow accepts an >+ optional parameter, `extendSelection`, for adding rows to the selection. >+ _selectedRowIndex is now used to track the most recently selected row. >+ This will be the only selected row unless multiple selection is enabled, >+ in which case it is the row that has the "focus", for purposes of selecting >+ a new row using the up or down arrow keys. >+ > 2018-09-17 Devin Rousso <drousso@apple.com> > > Web Inspector: generate CSSKeywordCompletions from backend values >diff --git a/Source/WebInspectorUI/UserInterface/Base/IndexSet.js b/Source/WebInspectorUI/UserInterface/Base/IndexSet.js >new file mode 100644 >index 0000000000000000000000000000000000000000..65b8cb24916783863dc7177beee21a4de1bb6973 >--- /dev/null >+++ b/Source/WebInspectorUI/UserInterface/Base/IndexSet.js >@@ -0,0 +1,152 @@ >+/* >+ * Copyright (C) 2018 Apple Inc. All Rights Reserved. >+ * >+ * Redistribution and use in source and binary forms, with or without >+ * modification, are permitted provided that the following conditions >+ * are met: >+ * 1. Redistributions of source code must retain the above copyright >+ * notice, this list of conditions and the following disclaimer. >+ * 2. Redistributions in binary form must reproduce the above copyright >+ * notice, this list of conditions and the following disclaimer in the >+ * documentation and/or other materials provided with the distribution. >+ * >+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' >+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, >+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR >+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS >+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR >+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF >+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS >+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN >+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) >+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF >+ * THE POSSIBILITY OF SUCH DAMAGE. >+ */ >+ >+WI.IndexSet = class IndexSet >+{ >+ constructor(values) >+ { >+ console.assert(Array.isArray(values) || !values); >+ >+ this._indexes = []; >+ >+ if (values) { >+ let previous = NaN; >+ for (let value of values.slice().sort((a, b) => a - b)) { >+ if (value === previous) >+ continue; >+ this._validateIndex(value); >+ this._indexes.push(value); >+ previous = value; >+ } >+ } >+ } >+ >+ // Public >+ >+ get size() { return this._indexes.length; } >+ >+ get firstIndex() >+ { >+ return this._indexes.length ? this._indexes[0] : NaN; >+ } >+ >+ get lastIndex() >+ { >+ return this._indexes.length ? this._indexes.lastValue : NaN; >+ } >+ >+ add(value) >+ { >+ this._validateIndex(value); >+ >+ let index = this._indexes.lowerBound(value); >+ if (this._indexes[index] === value) >+ return; >+ >+ this._indexes.insertAtIndex(value, index); >+ } >+ >+ delete(value) >+ { >+ this._validateIndex(value); >+ >+ if (!this.size) >+ return false; >+ >+ let index = this._indexes.lowerBound(value); >+ if (index === this._indexes.length) >+ return false; >+ this._indexes.splice(index, 1); >+ return true; >+ } >+ >+ has(value) >+ { >+ if (!this.size) >+ return false; >+ >+ let index = this._indexes.lowerBound(value); >+ return this._indexes[index] === value; >+ } >+ >+ clear() >+ { >+ this._indexes = []; >+ } >+ >+ indexGreaterThan(value) >+ { >+ const following = true; >+ return this._indexClosestTo(value, following); >+ } >+ >+ indexLessThan(value) >+ { >+ const following = false; >+ return this._indexClosestTo(value, following); >+ } >+ >+ [Symbol.iterator]() >+ { >+ return this._indexes[Symbol.iterator](); >+ } >+ >+ // Private >+ >+ _indexClosestTo(value, following) >+ { >+ if (!this.size) >+ return NaN; >+ >+ if (this.size === 1) { >+ if (following) >+ return this.firstIndex > value ? this.firstIndex : NaN; >+ return this.firstIndex < value ? this.firstIndex : NaN; >+ } >+ >+ let offset = following ? 1 : -1; >+ let position = this._indexes.lowerBound(value + offset); >+ >+ let closestIndex = this._indexes[position]; >+ if (closestIndex === undefined) >+ return NaN; >+ >+ if (value === closestIndex) >+ return NaN; >+ >+ if (!following && closestIndex > value) >+ return NaN; >+ return closestIndex; >+ } >+ >+ _validateIndex(value) >+ { >+ if (!Number.isInteger(value)) >+ throw new TypeError("Index must be an integer."); >+ >+ if (value < 0) >+ throw new RangeError("Index cannot be negative."); >+ } >+}; >diff --git a/Source/WebInspectorUI/UserInterface/Main.html b/Source/WebInspectorUI/UserInterface/Main.html >index e56ce4a050bfab3efd86f6a7f78eae23a822bcd2..3c5f83dbfed392335189649000460a9c3e600fe5 100644 >--- a/Source/WebInspectorUI/UserInterface/Main.html >+++ b/Source/WebInspectorUI/UserInterface/Main.html >@@ -278,6 +278,7 @@ > > <script src="Base/WebInspector.js"></script> > <script src="Base/Platform.js"></script> >+ <script src="Base/IndexSet.js"></script> > <script src="Base/LinkedList.js"></script> > <script src="Base/ListMultimap.js"></script> > <script src="Base/Object.js"></script> >diff --git a/Source/WebInspectorUI/UserInterface/Test.html b/Source/WebInspectorUI/UserInterface/Test.html >index 831ca73f239486ef72a0f1e69ab808f4bea3053d..d258e9d9a9b1942ac64c3a76aca22334d95a879d 100644 >--- a/Source/WebInspectorUI/UserInterface/Test.html >+++ b/Source/WebInspectorUI/UserInterface/Test.html >@@ -37,6 +37,7 @@ > > <script src="Base/WebInspector.js"></script> > <script src="Base/Platform.js"></script> >+ <script src="Base/IndexSet.js"></script> > <script src="Base/LinkedList.js"></script> > <script src="Base/ListMultimap.js"></script> > <script src="Base/Object.js"></script> >@@ -239,6 +240,10 @@ > <script src="Views/EditingSupport.js"></script> > <script src="Views/View.js"></script> > >+ <script src="Views/Resizer.js"></script> >+ <script src="Views/Table.js"></script> >+ <script src="Views/TableColumn.js"></script> >+ > <script type="text/javascript"> > WI.sharedApp = new WI.TestAppController; > WI.sharedApp.initialize(); >diff --git a/Source/WebInspectorUI/UserInterface/Test/Test.js b/Source/WebInspectorUI/UserInterface/Test/Test.js >index 6ef1e020006254dfe8af8275591d69a1ccd23ac7..9efe9cd687da164b92959c01c2edfca889c4caae 100644 >--- a/Source/WebInspectorUI/UserInterface/Test/Test.js >+++ b/Source/WebInspectorUI/UserInterface/Test/Test.js >@@ -104,6 +104,14 @@ WI.UIString = (string) => string; > > WI.indentString = () => " "; > >+WI.LayoutDirection = { >+ System: "system", >+ LTR: "ltr", >+ RTL: "rtl", >+}; >+ >+WI.resolvedLayoutDirection = () => { return InspectorFrontendHost.userInterfaceLayoutDirection(); } >+ > // Add stubs that are called by the frontend API. > WI.updateDockedState = () => {}; > WI.updateDockingAvailability = () => {}; >diff --git a/Source/WebInspectorUI/UserInterface/Views/DarkMode.css b/Source/WebInspectorUI/UserInterface/Views/DarkMode.css >index daabf7f1729f91b918ec4acfe77e0953b007bd8e..f826f995606a13a605efa1448fe32f36a1510125 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/DarkMode.css >+++ b/Source/WebInspectorUI/UserInterface/Views/DarkMode.css >@@ -837,6 +837,10 @@ > filter: invert(); > } > >+ .table > .data-container > .data-list > li.selected + li.selected { >+ border-top-color: var(--background-color); >+ } >+ > > /* ScopeBar.css */ > .scope-bar > li { >diff --git a/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js b/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js >index 070a5be94e0486ce5d095d2fc64b30b9416aaa88..2c40ad28e2d7538cdf6f93294fb45816c8bbfcb0 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js >+++ b/Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js >@@ -271,7 +271,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie > > if (this._table) { > this._selectedResource = null; >- this._table.clearSelectedRow(); >+ this._table.deselectAll(); > this._table.reloadData(); > this._hidePopover(); > this._hideResourceDetailView(); >@@ -285,7 +285,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie > let rowIndex = this._rowIndexForResource(representedObject); > if (rowIndex === -1) { > this._selectedResource = null; >- this._table.clearSelectedRow(); >+ this._table.deselectAll(); > this._hideResourceDetailView(); > return; > } >@@ -300,7 +300,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie > networkResourceDetailViewClose(resourceDetailView) > { > this._selectedResource = null; >- this._table.clearSelectedRow(); >+ this._table.deselectAll(); > this._hideResourceDetailView(); > } > >@@ -347,8 +347,9 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie > return column === this._nameColumn; > } > >- tableSelectedRowChanged(table, rowIndex) >+ tableSelectionDidChange(table) > { >+ let rowIndex = table.selectedRow; > if (isNaN(rowIndex)) { > this._selectedResource = null; > this._hideResourceDetailView(); >@@ -1403,7 +1404,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie > let rowIndex = this._rowIndexForResource(this._selectedResource); > if (rowIndex === -1) { > this._selectedResource = null; >- this._table.clearSelectedRow(); >+ this._table.deselectAll(); > return; > } > >diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.css b/Source/WebInspectorUI/UserInterface/Views/Table.css >index 61422947444c3929dc4bdac590ba45ba89c3b3f9..76a3405099101b7efa4e9cac69ff9bffa670beef 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/Table.css >+++ b/Source/WebInspectorUI/UserInterface/Views/Table.css >@@ -1,5 +1,5 @@ > /* >- * Copyright (C) 2013-2017 Apple Inc. All rights reserved. >+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved. > * > * Redistribution and use in source and binary forms, with or without > * modification, are permitted provided that the following conditions >@@ -126,6 +126,7 @@ body[dir=rtl] .table > .header > :matches(.sort-ascending, .sort-descending)::af > line-height: 20px; > vertical-align: middle; > white-space: nowrap; >+ border-top: 1px solid transparent; > } > > .table > .data-container > .data-list > li.selected { >@@ -133,6 +134,10 @@ body[dir=rtl] .table > .header > :matches(.sort-ascending, .sort-descending)::af > color: inherit !important; > } > >+.table > .data-container > .data-list > li.selected + li.selected { >+ border-top-color: white; >+} >+ > .table:focus > .data-container > .data-list li.selected { > background-color: var(--selected-background-color) !important; > color: var(--selected-foreground-color) !important; >diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.js b/Source/WebInspectorUI/UserInterface/Views/Table.js >index 839c06c7f158d8550cd5ec66fc9b3d365b2b3904..51941b11a601955d6fefc2c305b860e0cd7c1b5f 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/Table.js >+++ b/Source/WebInspectorUI/UserInterface/Views/Table.js >@@ -87,6 +87,8 @@ WI.Table = class Table extends WI.View > this._fillerHeight = 0; // Calculated in _resizeColumnsAndFiller. > > this._selectedRowIndex = NaN; >+ this._allowsMultipleSelection = false; >+ this._selectedRows = new WI.IndexSet; > > this._resizers = []; > this._currentResizer = null; >@@ -122,6 +124,12 @@ WI.Table = class Table extends WI.View > get delegate() { return this._delegate; } > get rowHeight() { return this._rowHeight; } > get selectedRow() { return this._selectedRowIndex; } >+ >+ get selectedRows() >+ { >+ return Array.from(this._selectedRows); >+ } >+ > get scrollContainer() { return this._scrollContainerElement; } > > get sortOrder() >@@ -200,6 +208,24 @@ WI.Table = class Table extends WI.View > this._dataSource.tableSortChanged(this); > } > >+ get allowsMultipleSelection() >+ { >+ return this._allowsMultipleSelection; >+ } >+ >+ set allowsMultipleSelection(flag) >+ { >+ if (this._allowsMultipleSelection === flag) >+ return; >+ >+ this._allowsMultipleSelection = flag; >+ >+ let numberOfSelectedRows = this._selectedRows.size; >+ this._selectedRows.clear(); >+ if (numberOfSelectedRows > 1) >+ this._notifySelectionDidChange(); >+ } >+ > resize() > { > this._cachedWidth = NaN; >@@ -211,6 +237,7 @@ WI.Table = class Table extends WI.View > reloadData() > { > this._cachedRows.clear(); >+ this._selectedRows.clear(); > > this._previousRevealedRowCount = NaN; > this.needsLayout(); >@@ -277,35 +304,71 @@ WI.Table = class Table extends WI.View > this._cachedRows.delete(rowIndex); > } > >- selectRow(rowIndex) >+ selectRow(rowIndex, extendSelection = false) > { >- if (this._selectedRowIndex === rowIndex) >+ console.assert(!extendSelection || this._allowsMultipleSelection, "Cannot extend selection with multiple selection disabled."); >+ >+ if (this._isRowSelected(rowIndex)) { >+ if (!extendSelection) >+ this._deselectAllAndSelect(rowIndex); > return; >+ } > >- let oldSelectedRow = this._cachedRows.get(this._selectedRowIndex); >- if (oldSelectedRow) >- oldSelectedRow.classList.remove("selected"); >+ if (!extendSelection && this._selectedRows.size) { >+ this._suppressNextSelectionDidChange = true; >+ this.deselectAll(); >+ } > > this._selectedRowIndex = rowIndex; >+ this._selectedRows.add(rowIndex); > > let newSelectedRow = this._cachedRows.get(this._selectedRowIndex); > if (newSelectedRow) > newSelectedRow.classList.add("selected"); > >- if (this._delegate.tableSelectedRowChanged) >- this._delegate.tableSelectedRowChanged(this, this._selectedRowIndex); >+ this._notifySelectionDidChange(); > } > >- clearSelectedRow() >+ deselectRow(rowIndex) > { >- if (isNaN(this._selectedRowIndex)) >+ if (!this._isRowSelected(rowIndex)) > return; > >- let oldSelectedRow = this._cachedRows.get(this._selectedRowIndex); >- if (oldSelectedRow) >- oldSelectedRow.classList.remove("selected"); >+ let oldSelectedRow = this._cachedRows.get(rowIndex); >+ if (!oldSelectedRow) >+ return; > >- this._selectedRowIndex = NaN; >+ oldSelectedRow.classList.remove("selected"); >+ >+ this._selectedRows.delete(rowIndex); >+ >+ if (this._selectedRowIndex === rowIndex) { >+ this._selectedRowIndex = NaN; >+ if (this._selectedRows.size) { >+ // Find selected row closest to deselected row. >+ let preceding = this._selectedRows.indexLessThan(rowIndex); >+ let following = this._selectedRows.indexGreaterThan(rowIndex); >+ >+ if (isNaN(preceding)) >+ this._selectedRowIndex = following; >+ else if (isNaN(following)) >+ this._selectedRowIndex = preceding; >+ else { >+ if ((following - rowIndex) < (rowIndex - preceding)) >+ this._selectedRowIndex = following; >+ else >+ this._selectedRowIndex = preceding; >+ } >+ } >+ } >+ >+ this._notifySelectionDidChange(); >+ } >+ >+ deselectAll() >+ { >+ const rowIndex = NaN; >+ this._deselectAllAndSelect(rowIndex); > } > > columnWithIdentifier(identifier) >@@ -678,7 +741,7 @@ WI.Table = class Table extends WI.View > let row = document.createElement("li"); > row.__index = rowIndex; > row.__widthGeneration = 0; >- if (rowIndex === this._selectedRowIndex) >+ if (this._isRowSelected(rowIndex)) > row.classList.add("selected"); > > this._cachedRows.set(rowIndex, row); >@@ -1201,10 +1264,19 @@ WI.Table = class Table extends WI.View > let column = this._visibleColumns[columnIndex]; > let rowIndex = row.__index; > >+ if (this._isRowSelected(rowIndex)) { >+ if (event.metaKey) >+ this.deselectRow(rowIndex) >+ else >+ this._deselectAllAndSelect(rowIndex); >+ return; >+ } >+ > if (this._delegate.tableShouldSelectRow && !this._delegate.tableShouldSelectRow(this, cell, column, rowIndex)) > return; > >- this.selectRow(rowIndex); >+ let extendSelection = event.metaKey && this._allowsMultipleSelection; >+ this.selectRow(rowIndex, extendSelection); > } > > _handleContextMenu(event) >@@ -1282,6 +1354,47 @@ WI.Table = class Table extends WI.View > }, checked); > } > } >+ >+ _deselectAllAndSelect(rowIndex) >+ { >+ if (!this._selectedRows.size) >+ return; >+ >+ if (this._selectedRows.size === 1 && this._selectedRows.firstIndex === rowIndex) >+ return; >+ >+ for (let selectedRowIndex of this._selectedRows) { >+ if (selectedRowIndex === rowIndex) >+ continue; >+ let oldSelectedRow = this._cachedRows.get(selectedRowIndex); >+ if (oldSelectedRow) >+ oldSelectedRow.classList.remove("selected"); >+ } >+ >+ this._selectedRowIndex = rowIndex; >+ this._selectedRows.clear(); >+ >+ if (!isNaN(rowIndex)) >+ this._selectedRows.add(rowIndex); >+ >+ this._notifySelectionDidChange(); >+ } >+ >+ _isRowSelected(rowIndex) >+ { >+ return this._selectedRows.has(rowIndex); >+ } >+ >+ _notifySelectionDidChange() >+ { >+ if (this._suppressNextSelectionDidChange) { >+ this._suppressNextSelectionDidChange = false; >+ return; >+ } >+ >+ if (this._delegate.tableSelectionDidChange) >+ this._delegate.tableSelectionDidChange(this); >+ } > }; > > WI.Table.SortOrder = { >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 0fc3099f8a6be2565a667cc2e1bbbeb4bdd9c680..2bf2b3576dbb51ab6fdc025a710ba2f586e36303 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,26 @@ >+2018-09-19 Matt Baker <mattbaker@apple.com> >+ >+ Web Inspector: Table should support multiple selection and Cmd-click behavior >+ https://bugs.webkit.org/show_bug.cgi?id=189705 >+ <rdar://problem/44571170> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * inspector/table/resources/table-utilities.js: Added. >+ (TestPage.registerInitializer.InspectorTest.TableDataSource): >+ (TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.get items): >+ (TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.tableNumberOfRows): >+ (TestPage.registerInitializer.InspectorTest.TableDelegate): >+ (TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tableSelectionDidChange): >+ (TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tablePopulateCell): >+ (TestPage.registerInitializer.InspectorTest.createTable): >+ (TestPage.registerInitializer): >+ >+ * inspector/table/table-selection-expected.txt: Added. >+ * inspector/table/table-selection.html: Added. >+ * inspector/unit-tests/index-set-expected.txt: Added. >+ * inspector/unit-tests/index-set.html: Added. >+ > 2018-09-19 Dawei Fenton <realdawei@apple.com> > > Marked imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-onnegotiationneeded.html as flak on iOS. >diff --git a/LayoutTests/inspector/table/resources/table-utilities.js b/LayoutTests/inspector/table/resources/table-utilities.js >new file mode 100644 >index 0000000000000000000000000000000000000000..b44606f7a21f8388ff538fc1c516da8028f86614 >--- /dev/null >+++ b/LayoutTests/inspector/table/resources/table-utilities.js >@@ -0,0 +1,65 @@ >+TestPage.registerInitializer(() => { >+ InspectorTest.TableDataSource = class TableDataSource >+ { >+ constructor(items) >+ { >+ this._items = items; >+ } >+ >+ get items() { return this._items; } >+ >+ // Table DataSource >+ >+ tableNumberOfRows(table) >+ { >+ return this._items.length; >+ } >+ } >+ >+ InspectorTest.TableDelegate = class TableDelegate >+ { >+ constructor(items) >+ { >+ this._items = items; >+ } >+ >+ tableSelectionDidChange(table) >+ { >+ InspectorTest.pass("Table selection changed."); >+ InspectorTest.dispatchEventToListeners("SelectionChanged"); >+ } >+ >+ tablePopulateCell(table, cell, column, rowIndex) >+ { >+ let item = this._items[rowIndex]; >+ InspectorTest.assert(item, "Should have an item for row " + rowIndex); >+ InspectorTest.assert(item[column.identifier], "Should have data for column " + column.identifier); >+ cell.textContent = item[column.identifier]; >+ return cell; >+ } >+ } >+ >+ InspectorTest.createTable = function(delegate, dataSource) { >+ if (!dataSource) { >+ let items = []; >+ for (let i = 0; i < 10; ++i) { >+ items.push({ >+ index: i, >+ name: "Row " + i, >+ }); >+ } >+ dataSource = new InspectorTest.TableDataSource(items); >+ } >+ >+ delegate = delegate || new InspectorTest.TableDelegate(dataSource.items); >+ >+ const rowHeight = 20; >+ let table = new WI.Table("test", dataSource, delegate, rowHeight); >+ table.addColumn(new WI.TableColumn("index", WI.UIString("Index"))); >+ table.addColumn(new WI.TableColumn("name", WI.UIString("Name"))); >+ >+ table.updateLayout(); >+ >+ return table; >+ } >+}); >diff --git a/LayoutTests/inspector/table/table-selection-expected.txt b/LayoutTests/inspector/table/table-selection-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..63045f8c239b93cf4685d600c745393b8ab1840d >--- /dev/null >+++ b/LayoutTests/inspector/table/table-selection-expected.txt >@@ -0,0 +1,68 @@ >+Tests for WI.Table. >+ >+ >+== Running test suite: Table >+-- Running test case: Table.constructor >+PASS: selectedRow should be NaN. >+PASS: Should have no selected rows. >+PASS: allowsMultipleSelection should initially be false. >+ >+-- Running test case: Table.SelectRow >+Selecting row 0. >+PASS: Table selection changed. >+PASS: selectedRow should be 0. >+PASS: selectedRows should be [0]. >+Selecting row 1. >+PASS: Table selection changed. >+PASS: selectedRow should be 1. >+PASS: selectedRows should be [1]. >+ >+-- Running test case: Table.DeselectRow >+Selecting row 0. >+PASS: Table selection changed. >+PASS: selectedRow should be 0. >+Deselecting row 0. >+PASS: Table selection changed. >+PASS: selectedRows should not include 0. >+PASS: Should have no selected rows. >+ >+-- Running test case: Table.AllowsMultipleSelection >+PASS: allowsMultipleSelection enabled. >+PASS: allowsMultipleSelection disabled. >+ >+-- Running test case: Table.SelectMultipleRows.ExtendSelection >+PASS: allowsMultipleSelection enabled. >+Selecting row 0. >+PASS: Table selection changed. >+PASS: selectedRow should be 0. >+Selecting row 1. >+PASS: Table selection changed. >+PASS: selectedRow should be 1. >+Selecting row 9. >+PASS: Table selection changed. >+PASS: selectedRow should be 9. >+PASS: selectedRows should be [0, 1, 9]. >+ >+-- Running test case: Table.SelectMultipleRows.SelectTheSameRowTwice.ExtendSelection >+Selecting row 0. >+PASS: Table selection changed. >+PASS: selectedRow should be 0. >+Selecting row 1. >+PASS: Table selection changed. >+PASS: selectedRow should be 1. >+Selecting row 1. >+PASS: selectedRow should be 1. >+PASS: selectedRows should be [0, 1]. >+ >+-- Running test case: Table.SelectMultipleRows.SelectTheSameRowTwice.NoExtendSelection >+Selecting row 0. >+PASS: Table selection changed. >+PASS: selectedRow should be 0. >+Selecting row 1. >+PASS: Table selection changed. >+PASS: selectedRow should be 1. >+Selecting row 1. >+PASS: Table selection changed. >+PASS: selectedRow should be 1. >+PASS: selectedRows should be [1]. >+ >diff --git a/LayoutTests/inspector/table/table-selection.html b/LayoutTests/inspector/table/table-selection.html >new file mode 100644 >index 0000000000000000000000000000000000000000..32ae6c4c08f2e8329c1f53aea2078f1343ea6646 >--- /dev/null >+++ b/LayoutTests/inspector/table/table-selection.html >@@ -0,0 +1,146 @@ >+<!doctype html> >+<html> >+<head> >+<script src="../../http/tests/inspector/resources/inspector-test.js"></script> >+<script src="resources/table-utilities.js"></script> >+<script> >+function test() >+{ >+ InspectorTest.redirectRequestAnimationFrame(); >+ >+ let suite = InspectorTest.createSyncSuite("Table"); >+ >+ // Import names. >+ let {createTable, cleanupTable} = InspectorTest; >+ >+ suite.addTestCase({ >+ name: "Table.constructor", >+ test() { >+ let table = createTable(); >+ >+ InspectorTest.expectThat(isNaN(table.selectedRow), "selectedRow should be NaN."); >+ InspectorTest.expectEqual(table.selectedRows.length, 0, "Should have no selected rows."); >+ InspectorTest.expectFalse(table.allowsMultipleSelection, "allowsMultipleSelection should initially be false."); >+ >+ return true; >+ } >+ }); >+ >+ function triggerSelectRow(table, rowIndex, extendSelection) { >+ InspectorTest.log(`Selecting row ${rowIndex}.`); >+ table.selectRow(rowIndex, extendSelection); >+ InspectorTest.expectEqual(table.selectedRow, rowIndex, `selectedRow should be ${rowIndex}.`); >+ } >+ >+ function triggerDeselectRow(table, rowIndex) { >+ InspectorTest.log(`Deselecting row ${rowIndex}.`); >+ table.deselectRow(rowIndex); >+ InspectorTest.expectFalse(table.selectedRows.includes(rowIndex), `selectedRows should not include ${rowIndex}.`); >+ } >+ >+ suite.addTestCase({ >+ name: "Table.SelectRow", >+ description: "Select a row, then select another row causing the frist to become deselected.", >+ test() { >+ let table = createTable(); >+ >+ triggerSelectRow(table, 0); >+ InspectorTest.expectShallowEqual(table.selectedRows, [0], "selectedRows should be [0]."); >+ triggerSelectRow(table, 1); >+ InspectorTest.expectShallowEqual(table.selectedRows, [1], "selectedRows should be [1]."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "Table.DeselectRow", >+ description: "Deselect the selected row.", >+ test() { >+ let table = createTable(); >+ >+ triggerSelectRow(table, 0); >+ triggerDeselectRow(table, 0); >+ InspectorTest.expectEqual(table.selectedRows.length, 0, "Should have no selected rows."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "Table.AllowsMultipleSelection", >+ description: "Should be able to enable multiple selection.", >+ test() { >+ let table = createTable(); >+ >+ table.allowsMultipleSelection = true; >+ InspectorTest.expectThat(table.allowsMultipleSelection, "allowsMultipleSelection enabled."); >+ table.allowsMultipleSelection = false; >+ InspectorTest.expectFalse(table.allowsMultipleSelection, "allowsMultipleSelection disabled."); >+ >+ return true; >+ } >+ }); >+ suite.addTestCase({ >+ name: "Table.SelectMultipleRows.ExtendSelection", >+ description: "Select multiple rows, extending the selection.", >+ test() { >+ let table = createTable(); >+ table.allowsMultipleSelection = true; >+ InspectorTest.expectThat(table.allowsMultipleSelection, "allowsMultipleSelection enabled."); >+ >+ const extendSelection = true; >+ >+ triggerSelectRow(table, 0, extendSelection); >+ triggerSelectRow(table, 1, extendSelection); >+ triggerSelectRow(table, 9, extendSelection); >+ InspectorTest.expectShallowEqual(table.selectedRows, [0, 1, 9], "selectedRows should be [0, 1, 9]."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "Table.SelectMultipleRows.SelectTheSameRowTwice.ExtendSelection", >+ description: "Select an already selected row, and extend the selection.", >+ test() { >+ let table = createTable(); >+ table.allowsMultipleSelection = true; >+ >+ const extendSelection = true; >+ >+ triggerSelectRow(table, 0, extendSelection); >+ triggerSelectRow(table, 1, extendSelection); >+ triggerSelectRow(table, 1, extendSelection); >+ InspectorTest.expectShallowEqual(table.selectedRows, [0, 1], "selectedRows should be [0, 1]."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "Table.SelectMultipleRows.SelectTheSameRowTwice.NoExtendSelection", >+ description: "Select an already selected row, and do not extend the selection.", >+ test() { >+ let table = createTable(); >+ table.allowsMultipleSelection = true; >+ >+ const extendSelection = true; >+ >+ triggerSelectRow(table, 0, extendSelection); >+ triggerSelectRow(table, 1, extendSelection); >+ triggerSelectRow(table, 1); >+ InspectorTest.expectShallowEqual(table.selectedRows, [1], "selectedRows should be [1]."); >+ >+ return true; >+ } >+ }); >+ >+ suite.runTestCasesAndFinish(); >+} >+</script> >+</head> >+<body onLoad="runTest()"> >+ <p>Tests for WI.Table.</p> >+</body> >+</html> >diff --git a/LayoutTests/inspector/unit-tests/index-set-expected.txt b/LayoutTests/inspector/unit-tests/index-set-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..b654c0df8cf9a3c13ffcf5a37284dc408f52e832 >--- /dev/null >+++ b/LayoutTests/inspector/unit-tests/index-set-expected.txt >@@ -0,0 +1,75 @@ >+Tests for WI.IndexSet. >+ >+ >+== Running test suite: IndexSet >+-- Running test case: IndexSet.constructor >+PASS: size should be zero. >+PASS: firstIndex should be NaN. >+PASS: lastIndex should be NaN. >+PASS: Should be []. >+ >+-- Running test case: IndexSet.constructor array >+Initialize IndexSet with an array. >+PASS: size should be 5. >+PASS: firstIndex should be 0. >+PASS: lastIndex should be 100. >+PASS: Should be [0,1,5,50,100]. >+Initialize IndexSet with an array containing duplicate indexes. >+PASS: size should be 5. >+PASS: Should be [0,1,5,50,100]. >+ >+-- Running test case: IndexSet.constructor invalid >+PASS: Should throw exception. >+RangeError: Index cannot be negative. >+ >+-- Running test case: IndexSet.prototype.clear >+PASS: size should be zero. >+PASS: firstIndex should be NaN. >+PASS: lastIndex should be NaN. >+PASS: Should be []. >+ >+-- Running test case: IndexSet.prototype.add >+PASS: size should be 1. >+PASS: has should return true. >+ >+-- Running test case: IndexSet.prototype.add duplicate >+PASS: size should be 1. >+ >+-- Running test case: IndexSet.prototype.add invalid >+PASS: Should throw exception. >+RangeError: Index cannot be negative. >+ >+-- Running test case: IndexSet.prototype.delete >+Given an IndexSet with values [1,2,3]: >+PASS: delete 3 should succeed. >+PASS: has 3 should return false. >+ >+-- Running test case: IndexSet.prototype.delete nonexistent >+Given an IndexSet with values [1,2,3]: >+PASS: delete 4 should fail. >+ >+-- Running test case: IndexSet.prototype.delete invalid >+PASS: Should throw exception. >+RangeError: Index cannot be negative. >+ >+-- Running test case: IndexSet.prototype[Symbol.iterator] >+Given an IndexSet with values [20,1,10,2]: >+1 >+2 >+10 >+20 >+ >+-- Running test case: IndexSet.prototype.indexGreaterThan >+Given an IndexSet with values [1,2]: >+PASS: Index greater than 0 should be 1. >+PASS: Index greater than 1 should be 2. >+PASS: Index greater than 2 should be NaN. >+PASS: Index greater than 3 should be NaN. >+ >+-- Running test case: IndexSet.prototype.indexLessThan >+Given an IndexSet with values [1,2]: >+PASS: Index less than 0 should be NaN. >+PASS: Index less than 1 should be NaN. >+PASS: Index less than 2 should be 1. >+PASS: Index less than 3 should be 2. >+ >diff --git a/LayoutTests/inspector/unit-tests/index-set.html b/LayoutTests/inspector/unit-tests/index-set.html >new file mode 100644 >index 0000000000000000000000000000000000000000..afb5bc7626212df852bf1f70be4f696cff9cfa1c >--- /dev/null >+++ b/LayoutTests/inspector/unit-tests/index-set.html >@@ -0,0 +1,232 @@ >+<!doctype html> >+<html> >+<head> >+<script src="../../http/tests/inspector/resources/inspector-test.js"></script> >+<script> >+function test() >+{ >+ let suite = InspectorTest.createSyncSuite("IndexSet"); >+ >+ function createIndexSet(values = []) { >+ if (!Array.isArray(values)) >+ values = [values]; >+ InspectorTest.log(`Given an IndexSet with values [${values}]:`); >+ >+ return new WI.IndexSet(values); >+ } >+ >+ // FIXME: Remove once <https://webkit.org/b/189255> is fixed. >+ // This provides a synchronous version of TestHarness.prototype.expectException. >+ function expectException(work) { >+ let error = null; >+ try { >+ work(); >+ InspectorTest.fail("Expected an exception."); >+ } catch (caughtError) { >+ error = caughtError; >+ } finally { >+ InspectorTest.expectNotNull(error, "Should throw exception."); >+ if (error) >+ InspectorTest.log(error.toString()); >+ } >+ } >+ >+ suite.addTestCase({ >+ name: "IndexSet.constructor", >+ test() { >+ let indexSet = new WI.IndexSet; >+ InspectorTest.expectEqual(indexSet.size, 0, "size should be zero."); >+ InspectorTest.expectThat(isNaN(indexSet.firstIndex), "firstIndex should be NaN."); >+ InspectorTest.expectThat(isNaN(indexSet.lastIndex), "lastIndex should be NaN."); >+ InspectorTest.expectShallowEqual(Array.from(indexSet), [], "Should be []."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.constructor array", >+ test() { >+ const values = [5, 1, 0, 100, 50]; >+ const sortedValues = values.slice().sort((a, b) => a - b); >+ >+ InspectorTest.log("Initialize IndexSet with an array."); >+ { >+ let indexSet = new WI.IndexSet(values); >+ InspectorTest.expectEqual(indexSet.size, values.length, `size should be ${values.length}.`); >+ InspectorTest.expectEqual(indexSet.firstIndex, sortedValues[0], `firstIndex should be ${sortedValues[0]}.`); >+ InspectorTest.expectEqual(indexSet.lastIndex, sortedValues.lastValue, `lastIndex should be ${sortedValues.lastValue}.`); >+ InspectorTest.expectShallowEqual(Array.from(indexSet), sortedValues, `Should be [${sortedValues}].`); >+ } >+ >+ InspectorTest.log("Initialize IndexSet with an array containing duplicate indexes."); >+ { >+ let indexSet = new WI.IndexSet(values.concat(values)); >+ InspectorTest.expectEqual(indexSet.size, values.length, `size should be ${values.length}.`); >+ InspectorTest.expectShallowEqual(Array.from(indexSet), sortedValues, `Should be [${sortedValues}].`); >+ } >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.constructor invalid", >+ test() { >+ expectException(() => { >+ new WI.IndexSet([-1]); >+ }); >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.clear", >+ test() { >+ let indexSet = new WI.IndexSet([1, 2, 3]); >+ indexSet.add(42); >+ indexSet.clear(); >+ InspectorTest.expectEqual(indexSet.size, 0, "size should be zero."); >+ InspectorTest.expectThat(isNaN(indexSet.firstIndex), "firstIndex should be NaN."); >+ InspectorTest.expectThat(isNaN(indexSet.lastIndex), "lastIndex should be NaN."); >+ InspectorTest.expectShallowEqual(Array.from(indexSet), [], "Should be []."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.add", >+ test() { >+ let indexSet = new WI.IndexSet; >+ indexSet.add(42); >+ InspectorTest.expectEqual(indexSet.size, 1, "size should be 1."); >+ InspectorTest.expectThat(indexSet.has(42), "has should return true."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.add duplicate", >+ test() { >+ let indexSet = new WI.IndexSet; >+ indexSet.add(42); >+ indexSet.add(42); >+ InspectorTest.expectEqual(indexSet.size, 1, "size should be 1."); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.add invalid", >+ test() { >+ let indexSet = new WI.IndexSet; >+ expectException(() => { >+ indexSet.add(-1); >+ }); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.delete", >+ test() { >+ let indexSet = createIndexSet([1, 2, 3]); >+ const indexToDelete = indexSet.lastIndex; >+ let wasDeleted = indexSet.delete(indexToDelete); >+ InspectorTest.expectThat(wasDeleted, `delete ${indexToDelete} should succeed.`); >+ InspectorTest.expectFalse(indexSet.has(indexToDelete), `has ${indexToDelete} should return false.`); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.delete nonexistent", >+ test() { >+ let indexSet = createIndexSet([1, 2, 3]); >+ const indexToDelete = indexSet.lastIndex + 1; >+ let wasDeleted = indexSet.delete(indexToDelete); >+ InspectorTest.expectFalse(wasDeleted, `delete ${indexToDelete} should fail.`); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.delete invalid", >+ test() { >+ let indexSet = new WI.IndexSet; >+ expectException(() => { >+ indexSet.delete(-1); >+ }); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype[Symbol.iterator]", >+ test() { >+ let indexSet = createIndexSet([20, 1, 10, 2]); >+ for (let index of indexSet) >+ InspectorTest.log(index); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.indexGreaterThan", >+ test() { >+ // Given an IndexSet with values [a, b]: >+ // a - 1 -> a >+ // a -> b >+ // b -> NaN >+ // b + 1 -> NaN >+ let indexSet = createIndexSet([1, 2]); >+ const {firstIndex, lastIndex} = indexSet; >+ const indexBefore = firstIndex - 1; >+ const indexAfter = lastIndex + 1; >+ InspectorTest.expectEqual(indexSet.indexGreaterThan(indexBefore), firstIndex, `Index greater than ${indexBefore} should be ${firstIndex}.`); >+ InspectorTest.expectEqual(indexSet.indexGreaterThan(firstIndex), lastIndex, `Index greater than ${firstIndex} should be ${lastIndex}.`); >+ InspectorTest.expectThat(isNaN(indexSet.indexGreaterThan(lastIndex)), `Index greater than ${lastIndex} should be NaN.`); >+ InspectorTest.expectThat(isNaN(indexSet.indexGreaterThan(indexAfter)), `Index greater than ${indexAfter} should be NaN.`); >+ >+ return true; >+ } >+ }); >+ >+ suite.addTestCase({ >+ name: "IndexSet.prototype.indexLessThan", >+ test() { >+ // Given an IndexSet with values [a, b]: >+ // a - 1 -> NaN >+ // a -> NaN >+ // b -> a >+ // b + 1 -> b >+ let indexSet = createIndexSet([1, 2]); >+ const {firstIndex, lastIndex} = indexSet; >+ const indexBefore = firstIndex - 1; >+ const indexAfter = lastIndex + 1; >+ >+ InspectorTest.expectThat(isNaN(indexSet.indexLessThan(indexBefore)), `Index less than ${indexBefore} should be NaN.`); >+ InspectorTest.expectThat(isNaN(indexSet.indexLessThan(firstIndex)), `Index less than ${firstIndex} should be NaN.`); >+ InspectorTest.expectEqual(indexSet.indexLessThan(lastIndex), firstIndex, `Index less than ${lastIndex} should be ${firstIndex}.`); >+ InspectorTest.expectEqual(indexSet.indexLessThan(indexAfter), lastIndex, `Index less than ${indexAfter} should be ${lastIndex}.`); >+ >+ return true; >+ } >+ }); >+ >+ suite.runTestCasesAndFinish(); >+} >+</script> >+</head> >+<body onLoad="runTest()"> >+ <p>Tests for WI.IndexSet.</p> >+</body> >+</html>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189705
:
350041
|
350049
|
350052
|
350055
|
350062
|
350154
|
350155
|
351031
|
351544
|
351560
|
351563
|
351621