Bug 193838 - Web Inspector: REGRESSION: selecting a path component with no siblings does nothing
Summary: Web Inspector: REGRESSION: selecting a path component with no siblings does n...
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-25 12:09 PST by Devin Rousso
Modified: 2019-02-26 16:00 PST (History)
3 users (show)

See Also:


Attachments
Patch (5.48 KB, patch)
2019-01-25 13:15 PST, Devin Rousso
drousso: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.