Bug 194169 – Web Inspector: REGRESSION: clicking a selected call frame doesn't re-scroll

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

# Steps to reproduce:
1. inspect <https://webkit.org>
2. set a breakpoint on global.js:2
3. reload the page
4. select the "Global Code" call frame
5. select the "(anonymous function)" call frame
6. scroll down so that the highlighted line isn't visible
7. re-select (e.g. click again) the "(anonymous function)" call frame

# Expected result:
The view should scroll to the line highlighted in step 6.

# Actual result:
The view doesn't scroll at all.

# Notes:
I believe this regressed with the addition of `WI.SelectionController`.  `WI.SelectionController.prototype.selectItem` early-returns if the index is already selected, meaning we don't call `selectionControllerSelectionDidChange` (which therefore doesn't fire `WI.TreeOutline.Event.SelectionDidChange`).

[Radar WebKit Bug Importer]

<rdar://problem/47743864>

[Matt Baker]

Created attachment 361082 [details]
Patch

[Devin Rousso]

Comment on attachment 361082 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361082&action=review

rs=me

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1018
> +        if (this.allowsRepeatSelection && !this.allowsMultipleSelection && treeElement.selected) {

Is there a reason/example why we don't want to have this behavior of multiple selection is enabled?  I think we should be a bit more "selective" (no pun intended), in that if we do allow multiple selection, but only have one item selected, re-selecting it would also fire an event.  Just because we have `allowsMultipleSelection`, doesn't mean we should blanket prevent this behavior.

[Matt Baker]

Created attachment 361107 [details]
Patch for landing

[Matt Baker]

Comment on attachment 361082 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361082&action=review

>> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1018
>> +        if (this.allowsRepeatSelection && !this.allowsMultipleSelection && treeElement.selected) {
> 
> Is there a reason/example why we don't want to have this behavior of multiple selection is enabled?  I think we should be a bit more "selective" (no pun intended), in that if we do allow multiple selection, but only have one item selected, re-selecting it would also fire an event.  Just because we have `allowsMultipleSelection`, doesn't mean we should blanket prevent this behavior.

I can't think of an example, but I don't see a reason why not. Instead of requiring that multiple selection be disabled, we just check that only one item is selected.

[WebKit Commit Bot]

Comment on attachment 361107 [details]
Patch for landing

Clearing flags on attachment: 361107

Committed r240947: <https://trac.webkit.org/changeset/240947>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.