To reproduce: 1. load any standalone SVG document 2. start web inspector 3. edit an attribute of an Element Expected: attribute should be changed Actual: ASSERTION FAILED: !node || node->isHTMLElement() /Users/rwlbuis/work/WebKit/Source/WebCore/html/HTMLElement.h(151) : WebCore::HTMLElement *WebCore::toHTMLElement(WebCore::Node *) 1 0x10cce2e0d WebCore::toHTMLElement(WebCore::Node*) 2 0x10d6a19c7 WebCore::InspectorDOMAgent::setAttributesAsText(WTF::String*, int, WTF::String const&, WTF::String const*) 3 0x10d6a1ed5 non-virtual thunk to WebCore::InspectorDOMAgent::setAttributesAsText(WTF::String*, int, WTF::String const&, WTF::String const*) 4 0x10d63e7e2 WebCore::InspectorBackendDispatcherImpl::DOM_setAttributesAsText(long, WebCore::InspectorObject*)
Created attachment 145467 [details] Patch
Comment on attachment 145467 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145467&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:584 > toHTMLElement(parsedElement.get())->setInnerHTML("<span " + text + "></span>", ec); Now that you use createHTMLElement, you don't need this cast.
Btw, you probably want a test to go with this change.
Hi, (In reply to comment #3) > Btw, you probably want a test to go with this change. Sorry, I did not see this comment. I'll try to come up with a testcase. Cheers, Rob.
Created attachment 145617 [details] Patch
Comment on attachment 145617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145617&action=review > LayoutTests/inspector/elements/set-attribute-non-html.svg:104 > + targetNode.setAttribute("foo", "foo2='baz2' foo3='baz3'"); You only want this test case to test your chanthis Btw, this patch is missing the expectations file, r- for this.
> You only want this test case to test your chanthis > *changes
Created attachment 145620 [details] Patch
Comment on attachment 145620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145620&action=review Thanks for adding the test! > LayoutTests/inspector/elements/set-attribute-non-html.svg:20 > + InspectorTest.addResult("=== Set attribute as text ==="); Why is this message not a part of the expectations? Dumping messages and tree below should end up there. It is added via inspector harness that adds divs under the body in the layout test page.
Hi Pavel, (In reply to comment #9) > (From update of attachment 145620 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145620&action=review > > Thanks for adding the test! No problem. > > LayoutTests/inspector/elements/set-attribute-non-html.svg:20 > > + InspectorTest.addResult("=== Set attribute as text ==="); > > Why is this message not a part of the expectations? Dumping messages and tree below should end up there. It is added via inspector harness that adds divs under the body in the layout test page. The situation is slightly more complicated :( The problem only occurs when the document in non HTML, so I took .svg as basis. However the underlying framework overall assumes document = HTML. So in output() document.createElement and document.body will not do the right thing for SVG. So either we need to make the framework take non HTML document into account or we accepts the fact that the testcase does not crash as good enough. What do you think? Cheers, Rob.
> What do you think? That sounds Ok. I would remove InspectorTest.addResult .dump* calls then to avoid the confusion.
Created attachment 145624 [details] Patch
Comment on attachment 145624 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145624&action=review > LayoutTests/inspector/elements/set-attribute-non-html.svg:17 > + targetNode.setAttribute("foo", "foo2='baz2' foo3='baz3'"); You should leave a callback and call next() to complete the suite execution. Otherwise it should time out.
(In reply to comment #13) > (From update of attachment 145624 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145624&action=review > > > LayoutTests/inspector/elements/set-attribute-non-html.svg:17 > > + targetNode.setAttribute("foo", "foo2='baz2' foo3='baz3'"); > > You should leave a callback and call next() to complete the suite execution. Otherwise it should time out. Sorry, new to this framework:) Will retry.
Created attachment 145636 [details] Patch
Comment on attachment 145636 [details] Patch Rejecting attachment 145636 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 Last 500 characters of output: return self.open(self.click(*args, **kwds)) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open return self._mech_open(url, data, timeout=timeout) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 255, in _mech_open raise response webkitpy.thirdparty.autoinstalled.mechanize._response.httperror_seek_wrapper: HTTP Error 500: Internal Server Error Full output: http://queues.webkit.org/results/12897426
Comment on attachment 145636 [details] Patch Clearing flags on attachment: 145636 Committed r119483: <http://trac.webkit.org/changeset/119483>
All reviewed patches have been landed. Closing bug.
(In reply to comment #18) > All reviewed patches have been landed. Closing bug. Seems to have worked the second time... Pavel, thanks for the patience and quick reviews!