Summary: | Web Inspector: RemoteObject.resolveNode should return a Promise | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Baker <mattbaker> | ||||||||
Component: | Web Inspector | Assignee: | Matt Baker <mattbaker> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Matt Baker
2017-09-16 18:11:01 PDT
Created attachment 321020 [details]
Patch
Comment on attachment 321020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321020&action=review > Source/WebInspectorUI/UserInterface/Protocol/RemoteObject.js:138 > - static resolveNode(node, objectGroup, callback) > + static resolveNode(node, objectGroup) > { > - DOMAgent.resolveNode(node.id, objectGroup, function(error, object) { > - if (!callback) > - return; > + return new Promise((resolve, reject) => { > + DOMAgent.resolveNode(node.id, objectGroup, function(error, object) { > + if (error || !object) { > + reject(error || "Could not resolve DOM node."); > + return; > + } > > - if (error || !object) > - callback(null); > - else > - callback(WI.RemoteObject.fromPayload(object, WI.mainTarget)); > + resolve(WI.RemoteObject.fromPayload(object, WI.mainTarget)); > + }); I'm a little confused by this. DOMAgent.resolveNode can produce a promise, lets use that promise (which handles errors) instead of creating our own: return DOMAgent.resolveNode(node.id, objectGroup) .then(({object}) => WI.RemoteObject.fromPayload(object, WI.mainTarget)); > Source/WebInspectorUI/UserInterface/Views/BoxModelDetailsSectionRow.js:408 > + function toggleInlineStyleProperty(property, value) { Name: inspectedPage_node_toggleInlineStyleProperty > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:313 > + function dimensions() { This should be named: inspectedPage_node_dimensions Since we are running this code on the `inspectedPage` and the this value is a `node`. > Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:508 > + function injectStyleAndToggleClass() { Same thing here: inspectedPage_node_injectStyleAndToggleClass Created attachment 321117 [details]
Patch
Comment on attachment 321117 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321117&action=review rs=me > Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:508 > + function injectStyleAndToggleClass() { Style: Name of this should have "inspectedPage_node_" prefix. Created attachment 321141 [details]
Patch for landing
Comment on attachment 321141 [details] Patch for landing Clearing flags on attachment: 321141 Committed r222188: <http://trac.webkit.org/changeset/222188> All reviewed patches have been landed. Closing bug. |