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.

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.

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

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 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

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