Summary: | Web Inspector: Cannot edit elements commented with <!-- | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexander Pavlov (apavlov) <apavlov> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2011-11-02 04:52:14 PDT
Created attachment 113471 [details]
[PATCH] Suggested solution
Comment on attachment 113471 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=113471&action=review r- for two branches in setOuterHTML > LayoutTests/inspector/elements/edit-dom-actions.html:165 > + if (typeof dataNode === "string") You should use different method for comment nodes. > Source/WebCore/inspector/InspectorDOMAgent.cpp:772 > + RefPtr<DocumentFragment> fragment = DocumentFragment::create(document); You should use this branch for both cases. > Source/WebCore/inspector/InspectorDOMAgent.cpp:776 > + return; Assign errorString here. > Source/WebCore/inspector/InspectorDOMAgent.cpp:781 > + return; ditto > Source/WebCore/inspector/InspectorDOMAgent.cpp:791 > + if (ec) ditto > Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1061 > + _populateCommentContextMenu: function(contextMenu, commentNode) You should call this method from _populateTagContextMenu (and call it _populateNodeContextMenu). Should also work for Text nodes. Created attachment 113515 [details]
[PATCH] Comments addressed
Comment on attachment 113515 [details] [PATCH] Comments addressed 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. Created attachment 113832 [details]
[PATCH] Comments addressed - 2
Comment on attachment 113832 [details] [PATCH] Comments addressed - 2 Clearing flags on attachment: 113832 Committed r99401: <http://trac.webkit.org/changeset/99401> All reviewed patches have been landed. Closing bug. Committed r99402: <http://trac.webkit.org/changeset/99402> |