[webkit-reviews] review denied: [Bug 77619] Web Inspector: Introduce "Copy XPath" popup menu item for DOM elements : [Attachment 125134] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 3 08:04:09 PST 2012


Vsevolod Vlasov <vsevik at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 77619: Web Inspector: Introduce "Copy XPath" popup menu item for DOM
elements
https://bugs.webkit.org/show_bug.cgi?id=77619

Attachment 125134: Patch
https://bugs.webkit.org/attachment.cgi?id=125134&action=review

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125134&action=review


> Source/WebCore/inspector/front-end/DOMAgent.js:539
> +    xPathValue: function(optimized)

Should be private as well as _xPathIndex()?

> Source/WebCore/inspector/front-end/DOMAgent.js:549
> +		   return new  WebInspector.DOMNode.XPathStep("//*[@id=\"" +
this.getAttribute("id") + "\"]", true);

double space between new and WebInspector.

> Source/WebCore/inspector/front-end/DOMAgent.js:580
> +	* @return -1 in case of error, 0 if no siblings matching the same
expression, <XPath index among the same expression-matching sibling nodes>
otherwise.

Please add type annotation here and move description to a separate comment
inside function.

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1710
> +	   this.representedObject.copyXPath(true);

Please add this argument to copyXPath() method, r- for that.


More information about the webkit-reviews mailing list