Bug 230287 – Web Inspector: `TreeOutline` should return early when failing to find an ancestor while populating the tree

• Component: Web Inspector
• Status: RESOLVED FIXED

[Patrick Angle]

.

[Radar WebKit Bug Importer]

<rdar://problem/83125960>

[Patrick Angle]

Created attachment 438201 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 438201 [details]
Patch v1.0

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

r=me, good catch :)

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:518
>              item = this.findTreeElement(ancestors[i], isAncestor, getParent);

Aside: I wonder if maybe this should just be `this.getCachedTreeElement(ancestors[i])`, as there's really no point to walking up the `getParent` chain again since we've already done that.  Tho you might need to duplicate the bit that iterates over `this.children`, but conceptually if we have an item inside `this.children` then it should already be cached.

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:520
> +                return;

this should be `return null;` to match the other `return`

[Devin Rousso]

Comment on attachment 438201 [details]
Patch v1.0

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

>> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:518
>>              item = this.findTreeElement(ancestors[i], isAncestor, getParent);
> 
> Aside: I wonder if maybe this should just be `this.getCachedTreeElement(ancestors[i])`, as there's really no point to walking up the `getParent` chain again since we've already done that.  Tho you might need to duplicate the bit that iterates over `this.children`, but conceptually if we have an item inside `this.children` then it should already be cached.

Aside: we may also wanna add an early-return above the `while` if `getParent` is NOT provided, as right now it's kinda this silent assumption 😅

[Patrick Angle]

Created attachment 438291 [details]
Patch v1.1

[EWS]

Committed r282477 (241725@main): <https://commits.webkit.org/241725@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438291 [details].