Bug 208679 - REGRESSION(r253759): Web Inspector: Audits: results folder doesn't get selected by pressing Arrow Down key
Summary: REGRESSION(r253759): Web Inspector: Audits: results folder doesn't get select...
Status: RESOLVED FIXED
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: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-05 17:21 PST by Nikita Vasilyev
Modified: 2020-03-10 18:36 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.78 KB, patch)
2020-03-06 11:08 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2020-03-05 17:21:12 PST
# STEPS TO REPRODUCE:
1. inspect any page
2. go to the Audit Tab
3. run any audit
4. select any audit
5. keep pressing ↓ to eventually select the Results folder
=> folder isn't actually selected, but instead a focus outline can be seen
6. press ↓ again
=> selection jumps to the first item in the list
Comment 1 Radar WebKit Bug Importer 2020-03-05 17:21:22 PST
<rdar://problem/60108942>
Comment 2 Nikita Vasilyev 2020-03-05 18:39:40 PST
A combination of a couple of odd things cause this bug.

_dispatchSelectionDidChangeEvent gets called twice. First time from SelectionController:

	_dispatchSelectionDidChangeEvent (TreeOutline.js:1131)
	selectionControllerSelectionDidChange (TreeOutline.js:811)
	_updateSelectedItems (SelectionController.js:392)
	selectItem (SelectionController.js:97)
	_selectItemsFromArrowKey (SelectionController.js:334)
	handleKeyDown (SelectionController.js:229)
	_treeKeyDown (TreeOutline.js:669)
	_treeKeyDown

Second time directly from TreeOutline:

	_dispatchSelectionDidChangeEvent (TreeOutline.js:1131)
	selectTreeElementInternal (TreeOutline.js:897)
	select (TreeElement.js:542)
	_treeKeyDown (TreeOutline.js:682)
	_treeKeyDown

The second call results in ContentViewContainer.prototype.showDefaultContentView:

	showDefaultContentView (AuditNavigationSidebarPanel.js:71)
	_treeSelectionDidChange (AuditNavigationSidebarPanel.js:287)
	dispatch (Object.js:165)
	dispatchEventToListeners (Object.js:172)
	_dispatchSelectionDidChangeEvent (TreeOutline.js:1139)
	selectTreeElementInternal (TreeOutline.js:897)
	select (TreeElement.js:542)
	_treeKeyDown (TreeOutline.js:682)
	_treeKeyDown

Which resets treeElement selection. Thus this bug happens.
Comment 3 Nikita Vasilyev 2020-03-06 00:33:32 PST
This didn't regress with r257380.
Comment 4 Nikita Vasilyev 2020-03-06 01:14:41 PST
This broken more than 6 weeks ago. My recent AXI changes had nothing to do with it.

Works in r255037
Broken in r252338
http://trac.webkit.org/log/trunk/?mode=follow_copy&rev=255037&stop_rev=252338

A lot of builds in this range are broken so I'm stopping bisecting for now.
Comment 5 Nikita Vasilyev 2020-03-06 01:39:10 PST
This broke in Bug 205435 - Web Inspector: Audit: exiting edit mode with the same selection as before entering edit mode doesn't reselect
https://trac.webkit.org/changeset/253759/webkit
Comment 6 Devin Rousso 2020-03-06 11:08:25 PST
Created attachment 392743 [details]
Patch
Comment 7 Nikita Vasilyev 2020-03-06 14:22:21 PST
Comment on attachment 392743 [details]
Patch

I think it's fine to duct tape this for now. Ultimately, when allowsRepeatSelection is true, WI.TreeOutline.Event.SelectionDidChange shouldn't fire twice when selection only changes once.
Comment 8 WebKit Commit Bot 2020-03-10 18:36:01 PDT
Comment on attachment 392743 [details]
Patch

Clearing flags on attachment: 392743

Committed r258246: <https://trac.webkit.org/changeset/258246>
Comment 9 WebKit Commit Bot 2020-03-10 18:36:03 PDT
All reviewed patches have been landed.  Closing bug.