Bug 193838

Summary:

Web Inspector: REGRESSION: selecting a path component with no siblings does nothing

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED WORKSFORME

Severity:

Normal

CC:

hi, inspector-bugzilla-changes, mattbaker

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

See Also:

https://bugs.webkit.org/show_bug.cgi?id=194169

Attachments:

Description	Flags
Patch	none

Description Devin Rousso 2019-01-25 12:09:53 PST

# STEPS TO REPRODUCE:
1. go to <https://devinrousso.com/demo/WebKit/canvas.html>
2. open the Canvas tab
3. select a Canvas "Card"
4. try to click the "Overview" path component

Comment 1 Devin Rousso 2019-01-25 13:15:43 PST

Created attachment 360150 [details]
Patch

Comment 2 Matt Baker 2019-01-25 13:37:21 PST

Comment on attachment 360150 [details]
Patch

r-, as I don't think removing allowsRepeatSelection is the right fix for this.

We rely on allowsRepeatSelection in the Debugger tab Call Stack tree. Selecting an already selected element should scroll the source code location into view. This change breaks that behavior.

Comment 3 Devin Rousso 2019-02-01 10:56:15 PST

Comment on attachment 360150 [details]
Patch

(In reply to Matt Baker from comment #2)
> We rely on allowsRepeatSelection in the Debugger tab Call Stack tree.
> Selecting an already selected element should scroll the source code location
> into view. This change breaks that behavior.
This is currently not working for me on ToT.  I don't think that issue has anything to do with this.  If anything, this change should help that, because by removing `allowsRepeatSelection`, we're effectively setting it to `true` for all `WI.TreeOutline`, so we should _always_ be able to reselect (and therefore re-scroll).  Resetting to r?.

Comment 4 Devin Rousso 2019-02-01 11:39:46 PST

(In reply to Devin Rousso from comment #3)
> (In reply to Matt Baker from comment #2)
> > We rely on allowsRepeatSelection in the Debugger tab Call Stack tree. Selecting an already selected element should scroll the source code location into view. This change breaks that behavior.
> This is currently not working for me on ToT.  I don't think that issue has anything to do with this.  If anything, this change should help that, because by removing `allowsRepeatSelection`, we're effectively setting it to `true` for all `WI.TreeOutline`, so we should _always_ be able to reselect (and therefore re-scroll).  Resetting to r?.
I believe this functionality 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`).  Because of these changes, setting `allowsRepeatSelection` doesn't actually do anything.

Unfortunately, attachment 360150 [details] doesn't fix that issue :(

Comment 5 Matt Baker 2019-02-26 16:00:34 PST

I can no longer reproduce this.