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
View All Add an attachment (proposed patch, testcase, etc.)

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.