Web Inspector: REGRESSION(r238602): Elements: collapsing a DOM node with the left arrow doesn't work

[Devin Rousso]

The issue is intermittent, but it happens often, and when it does it completely breaks navigation.  My guess is that some state is getting stuck somewhere.

In the attached video, any "pause" where no navigation/clicking occurs is me mashing the left arrow button 😅

[Devin Rousso]

Created attachment 356466 [details]
[Video] Screenrecording of issue

[Radar WebKit Bug Importer]

<rdar://problem/46455019>

[Radar WebKit Bug Importer]

<rdar://problem/46455020>

[Matt Baker]

Created attachment 356543 [details]
Patch

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #1)
> Created attachment 356466 [details]
> [Video] Screenrecording of issue

By the way, Keycastr can visualize key presses. Otherwise, I don't know when you pressed left arrow key and it didn't work.

https://github.com/keycastr/keycastr

[Nikita Vasilyev]

I was able to reproduce the bug. With the patch applied, I could no longer reproduce it!

Trying to understand what it actually does now.

[Nikita Vasilyev]

Comment on attachment 356543 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:12
> +        Don't early return when the element is not the selected tree element.
> +        This condition no longer holds now that TreeOutline supports multiple selection.

This makes sense. Looks good to me.

[EWS Watchlist]

Comment on attachment 356543 [details]
Patch

Attachment 356543 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10270768

New failing tests:
http/tests/misc/resource-timing-resolution.html

[EWS Watchlist]

Created attachment 356560 [details]
Archive of layout-test-results from ews203 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews203  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Devin Rousso]

Comment on attachment 356543 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/TreeElement.js:535
> +        if (!this.treeOutline || !this.selected)

We should add an assert here, just so that we can observe other non-multiple-selection callers and make sure we aren't generating any new issues:

    console.assert(this.treeOutline.allowsMultipleSelection || this.treeOutline.selectedTreeElement === this);

or even:

    console.assert(this.treeOutline.selectedTreeElements.includes(this));

[Matt Baker]

Created attachment 356676 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 356676 [details]:

inspector/console/js-isLikelyStackTrace.html bug 192440 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[EWS Watchlist]

Comment on attachment 356676 [details]
Patch for landing

Attachment 356676 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10285556

New failing tests:
http/tests/misc/resource-timing-resolution.html

[EWS Watchlist]

Created attachment 356683 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[WebKit Commit Bot]

Comment on attachment 356676 [details]
Patch for landing

Clearing flags on attachment: 356676

Committed r238938: <https://trac.webkit.org/changeset/238938>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.