1830111 - NS_ERROR_ILLEGAL_VALUE when opening inspector with uBlock installed

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce
0. Make sure you have uBlock installed

1. Navigate to https://ffx-devtools-ublock-markup-view.glitch.me/
2. Open the inspector
3. In the search input, type head

Expected results

I get some autocompletion/result since there's a header item

Actual results

Nothing happens in the UI, in the Browser Console I can see the following error:

 Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [inIDeepTreeWalker.currentNode] from: server0.conn0.windowGlobal6442450945/domwalker24 (resource://devtools/server/actors/inspector/document-walker.js:69:0)

---

112 is not impacted, 113 is.

Looks like walker.currentNode setter is throwing when it's given this iframe <iframe id="google_ads_frame0">

Regressor might be Bug 1825825
Would you know what's happening Emilio?

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

I updated https://ffx-devtools-ublock-markup-view.glitch.me/ so you don't need uBlock anymore.
The issue is triggered by an iframe (without src) onto which we append another iframe with

      document
        .querySelector("iframe")
        .appendChild(document.createElement("iframe"));

With this simpler page, I can reproduce the issue on ESR, so Bug 1825825 is not concerned

Comment 2

[Alexandre Poirot [:ochameau]]

I suspect it relates to this code:
https://searchfox.org/mozilla-central/rev/5c922d8b93b43c18bf65539bfc72a30f84989003/layout/inspector/inDeepTreeWalker.cpp#169-177

    GetChildren(*parent, mSiblings);
    // We cached all the siblings (if there are any) of the current node, but we
    // still have to set the index too, to be able to iterate over them.
    int32_t index = mSiblings.IndexOf(mCurrentNode);
    if (index < 0) {
      // If someone tries to set current node to some value that is not
      // reachable otherwise, let's throw. (For example mShowAnonymousContent is
      // false and some NAC was passed in).
      // Restore state first.
      mCurrentNode = std::move(oldCurrent);
      oldSiblings.SwapElements(mSiblings);
      return NS_ERROR_INVALID_ARG;
    }

The iframe added as a child of another iframe tricks the DOM Tree as it doesn't seem to be considered as a legit children of an iframe.
At least, it is consistant with the current behavior of the inspector.
We only shows iframe's inner document and none of the DOM Element added as children like comment 1 snippet.

On top of this, it is really an edge case as it really relates to only having iframe within iframes.
The following page fails:
data:text/html,<iframe></iframe><script> document.querySelector("iframe").appendChild(document.createElement("iframe"));</script>
But this one does not:
data:text/html,<iframe></iframe><script> document.querySelector("iframe").appendChild(document.createElement("div"));</script>

That's because this inDeepTreeWalker::setCurrentNode is only used with the problematic iframe when we are calling getEmbedderElement from there:
https://searchfox.org/mozilla-central/rev/5c922d8b93b43c18bf65539bfc72a30f84989003/devtools/client/fronts/walker.js#405

Should we somehow be able to display such iframe in the DOM Tree?
In the meantime, I think we should only fix the assertion and be consistant by ignoring these edgecases.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1830111 - [devtools] Ignore exception related to nested, invisible iframes.

You can add an <iframe> as an immediate children of another <iframe>.
This was causing exception when the walker was processing such nested iframe.
inDeepTreeWalker::SetCurrentNode is throwing because such nested iframe
aren't considered as valid children, even if they are visible via topFrame.children.

Let's ignore this exception as the inspector wouldn't show such iframe anyway.
We only show the inner document for <iframe> and none of the possibly manually added children.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8295c1c7bfa0
[devtools] Ignore exception related to nested, invisible iframes. r=devtools-reviewers,nchevobbe

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/8295c1c7bfa0

Comment 6

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-05-08 Nightly build on macOS 12.
Verified as fixed on Firefox 115.0b5(build ID: 20230613195413) and Nightly 116.0a1(build ID: 20230613152538) on macOS 12, Windows 10, Ubuntu 22.