[webkit-reviews] review granted: [Bug 176827] Web Inspector: move Console.addInspectedNode to DOM.setInspectedNode : [Attachment 321116] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<webkit at devinrousso.com>'s request for review:
Bug 176827: Web Inspector: move Console.addInspectedNode to
DOM.setInspectedNode
https://bugs.webkit.org/show_bug.cgi?id=176827

Attachment 321116: Patch

https://bugs.webkit.org/attachment.cgi?id=321116&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 321116
  --> https://bugs.webkit.org/attachment.cgi?id=321116
Patch

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

r=me, very nice!

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:61
> +    get inspectedNode() { return this._inspectedNode; }

Is this actually used? inspectedNode/_inspectedNode other than an early return
in `setInspectedNode()`?

Given that `setInspectedNode` doesn't populate it until a callback, I'd expect
clients to have a hard time using this, since it would be wrong immediately
after someone sets the value. I think we should drop this.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:537
> +	   // COMPATIBILITY (iOS 11): DOM.setInspectedNode did not exist.
> +	   if (!DOMAgent.setInspectedNode) {

Well done!