[webkit-reviews] review denied: [Bug 71357] Web Inspector: Cannot edit elements commented with <!-- : [Attachment 113515] [PATCH] Comments addressed

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Nov 5 09:05:47 PDT 2011


Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 71357: Web Inspector: Cannot edit elements commented with <!--
https://bugs.webkit.org/show_bug.cgi?id=71357

Attachment 113515: [PATCH] Comments addressed
https://bugs.webkit.org/attachment.cgi?id=113515&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113515&action=review


> Source/WebCore/inspector/InspectorDOMAgent.cpp:751
>      HTMLElement* element = assertHTMLElement(errorString, nodeId);

If user passes TextNode id into this method, he will get an error message
saying that it should always be id of the Element. But we do support nodes (see
comment above). You should either support more node types and remove this
assertion or not call getOuterHTML for non-Element nodes.

> Source/WebCore/inspector/InspectorDOMAgent.cpp:772
> +    if (document->isHTMLDocument())

You could put this check earlier. Can this happen at all?

> Source/WebCore/inspector/InspectorDOMAgent.cpp:787
> +	   requiresTotalUpdate = htmlElement->tagName() == "HTML" ||
htmlElement->tagName() == "BODY" || htmlElement->tagName() == "HEAD";

no need to cast, tagName() is nodeName() for elements.


More information about the webkit-reviews mailing list