Bugzilla

(In reply to Julian Descottes [:jdescottes] from comment #4)
> As we can see in the stack trace, the call to querySelector comes from the `async findNodeFront(nodeSelectors)` method (also in the walker front). The idea in `findNodeFront` is that if we have several selectors, it means the selection is nested in an iframe. And therefore, in order to get the "final" node front, for each selector but the last we do:
> - get the iframe node front
> - wait for iframe to be loaded
> - go to the next selector
> 
> (https://searchfox.org/mozilla-central/rev/f66eb70d175a2017a74b4f61ed556d5835c8a9db/devtools/shared/fronts/walker.js#514-537)
> 
> But what if the iframe is missing when we reload? There's nothing in the code to handle this, we assume the selector will return a valid node front. JSFiddle most likely creates the output iframe after the page was loaded, and we try to reselect the node _before_ the iframe is created. So we can't find the iframe node front, and the code crashes. I think the only thing to do here is to bail out if we can't find a node front. We can't possibly wait for all iframes to be dynamically created after a reload (some might even never be recreated!) 
> 

Thank you Julian for the thorough explanation of how this part of the code works. I really appreciate it. I think bailing out when the node front is missing is the best way to go and I think it's safe to assume returning just the root node will suffice? 

> The fact that the response only contains "from" is because the walker actor's `querySelector` returns null when the node can't be found (https://searchfox.org/mozilla-central/rev/f66eb70d175a2017a74b4f61ed556d5835c8a9db/devtools/server/actors/inspector/walker.js#1220).
> 
>  This is indeed confusing because it can lead to think that the response is "wrong". I am not 100% sure why protocol.js allows this, because if you look at the walker specs, the return value of querySelector is `disconnectedNode`:
> ```javascript
>     querySelector: {
>       request: {
>         node: Arg(0, "domnode"),
>         selector: Arg(1),
>       },
>       response: RetVal("disconnectedNode"),
>     },
> ```
> Normally, nullable types are marked as `nullable`, so I would have expected to see `nullable:disconnectedNode` here. But maybe protocol.js allows all responses to take a null value. Anyway we could change that by returning `{ node: null, newParents: [] }` in `querySelector`, in the actor implementation. That won't change the crash here, but that would make the investigation less confusing (note that you will have to update the specs of `disconnectedNode` to allow `node` to take null values, otherwise protocol.js will complain).
> 
> I would suggest to fix this in a follow up bug.

Sounds good. I'll create the appropriate issue for this.

(In reply to Julian Descottes [:jdescottes] from comment #4)
> As we can see in the stack trace, the call to querySelector comes from the `async findNodeFront(nodeSelectors)` method (also in the walker front). The idea in `findNodeFront` is that if we have several selectors, it means the selection is nested in an iframe. And therefore, in order to get the "final" node front, for each selector but the last we do:
> - get the iframe node front
> - wait for iframe to be loaded
> - go to the next selector
> 
> (https://searchfox.org/mozilla-central/rev/f66eb70d175a2017a74b4f61ed556d5835c8a9db/devtools/shared/fronts/walker.js#514-537)
> 
> But what if the iframe is missing when we reload? There's nothing in the code to handle this, we assume the selector will return a valid node front. JSFiddle most likely creates the output iframe after the page was loaded, and we try to reselect the node _before_ the iframe is created. So we can't find the iframe node front, and the code crashes. I think the only thing to do here is to bail out if we can't find a node front. We can't possibly wait for all iframes to be dynamically created after a reload (some might even never be recreated!) 
> 

Thank you Julian for the thorough explanation of how this part of the code works. I really appreciate it. I think bailing out when the node front is missing is the best way to go and I think it's safe to assume returning just the root node will suffice? 

> The fact that the response only contains "from" is because the walker actor's `querySelector` returns null when the node can't be found (https://searchfox.org/mozilla-central/rev/f66eb70d175a2017a74b4f61ed556d5835c8a9db/devtools/server/actors/inspector/walker.js#1220).
> 
>  This is indeed confusing because it can lead to think that the response is "wrong". I am not 100% sure why protocol.js allows this, because if you look at the walker specs, the return value of querySelector is `disconnectedNode`:
> ```javascript
>     querySelector: {
>       request: {
>         node: Arg(0, "domnode"),
>         selector: Arg(1),
>       },
>       response: RetVal("disconnectedNode"),
>     },
> ```
> Normally, nullable types are marked as `nullable`, so I would have expected to see `nullable:disconnectedNode` here. But maybe protocol.js allows all responses to take a null value. Anyway we could change that by returning `{ node: null, newParents: [] }` in `querySelector`, in the actor implementation. That won't change the crash here, but that would make the investigation less confusing (note that you will have to update the specs of `disconnectedNode` to allow `node` to take null values, otherwise protocol.js will complain).
> 
> I would suggest to fix this in a follow up bug.

Sounds good. I've created the appropriate issue for this:  Bug 1601003