914077 - While using the inspector on B2G: console.warn: Tried to use rawNode on a remote connection

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Comment 1

[Dave Camp (:dcamp)]

I put that warning in to help identify code that was expecting raw nodes and not dealing well with them.  Now there are two places .rawNode is used:

a) The markupview checks for local nodes because we don't do tag renaming remotely yet
b) The computed-view checks for local nodes when clicking on a link to give view-source extra context if possible.

These are both well-behaved, so I see two possibilities:

1) Just remove the warning.
2) Add an isLocal() thing to the front that well-behaved code can use to protect itself from the warning.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

This doesn't seem like a critical fix to me for App Manager v1.  Though it's probably quick to fix too... but there are many other things to do.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

This also happens when debugging Thunderbird. As mentioned, not really critical though.

Comment 4

[Paul Rouget [:paul]]

Attachment: v1

Comment 5

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=6ee75940f7cb

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 825317 [details] [diff] [review]
v1

Review of attachment 825317 [details] [diff] [review]:
-----------------------------------------------------------------

It will be great to not see the warnings anymore! Here are a couple of other places I found rawNode() accessed that should be updated as well: 

* https://mxr.mozilla.org/mozilla-central/source/browser/devtools/styleinspector/computed-view.js#1220
* https://mxr.mozilla.org/mozilla-central/source/browser/devtools/inspector/selection.js#107

::: browser/devtools/inspector/selection.js
@@ +210,5 @@
>      }
>  
>      // As long as there are still tools going around
>      // accessing node.rawNode, this needs to stay.
> +    if (!node.doesUseRemoteConnection_toBeDeprecated()) {

This logic is actually subtly different than before.  Even in the case of a non-remote node, there is still a chance that rawNode() will return null.  Before, it would skip this condition and continue into the rest of the function, but now it will return false all the time after entering.

I'm guessing that the instance where this is the case it may have ended up returning false anyway, but just something to be aware of - may want to do as you did before with: 

let rawNode = null;
if (node.doesUseRemoteConnection_toBeDeprecated()) {
    rawNode = node.rawNode();
}
if (rawNode) ...

::: toolkit/devtools/server/actors/inspector.js
@@ +563,5 @@
> +   *
> +   * This will, one day, be removed. External code should
> +   * not need to know if the target is remote or not.
> +   */
> +  doesUseRemoteConnection_toBeDeprecated: function() {

Maybe we could rename / invert this to isLocal() to be consistent with the WalkerFront: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/inspector/selection.js#199.  Or maybe rename that method to match this one (it is only accessed in one place for the walker).

Comment 7

[Paul Rouget [:paul]]

Attachment: v2

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 827233 [details] [diff] [review]
v2

Review of attachment 827233 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r+ with a green try

::: browser/devtools/styleinspector/computed-view.js
@@ +1216,5 @@
>        return;
>      }
>  
>      let contentDoc = null;
> +    if (node.isLocal_toBeDeprecated()) {

Is node defined here?  Thinking this should be this.tree.viewedElement.isLocal_toBeDeprecated.

::: toolkit/devtools/server/actors/inspector.js
@@ +558,5 @@
>    },
>  
>    /**
> +   * Do we use a local target?
> +   * Usefull to know if a rawNode is available or not.

s/Usefull/Useful

Comment 9

[Paul Rouget [:paul]]

Attachment: v2.1

Addressed comments. https://tbpl.mozilla.org/?tree=Try&rev=eb1f689862bb

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/0d3837a1ce9d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0d3837a1ce9d