[webkit-reviews] review granted: [Bug 194299] REGRESSION: Elements tab: Uncaught Exception: No node with given id found : [Attachment 363703] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 5 22:11:12 PST 2019

Devin Rousso <drousso at apple.com> has granted Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 194299: REGRESSION: Elements tab: Uncaught Exception: No node with given id

Attachment 363703: Patch


--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 363703
  --> https://bugs.webkit.org/attachment.cgi?id=363703

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


> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:307
> +	   this._treeElementsToRemove.sort((a, b) => getLevel(b) -

Should we sort before calling `removeSelectedItems`?  It should make
performance slightly better since it organizes the array to be bottom-up based
on the DOM and we check bottom-up inside `canSelectTreeElement`, meaning that
the "lower" nodes are more likely to be earlier on in the array.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:330
> +	       return super.canSelectTreeElement(treeElement);

Rather than only using this path if we aren't removing, I think we should flip
this around.
    if (!super.canSelectTreeElement(treeElement))
	return false;

    let willRemoveAncestorOrSelf = false;
    if (this._treeElementsToRemove) {
	while (treeElement && !willRemoveAncestorOrSelf) {
	    willRemoveAncestorOrSelf =
	    treeElement = treeElement.parent;
    return !willRemoveAncestorOrSelf;

More information about the webkit-reviews mailing list