Web Inspector: Second call to setAttributeNS creates non-prefixed attribute

• Component: Web Inspector
• Status: RESOLVED FIXED

Created attachment 375081 [details]
Example.

Calling setAttributeNS creates a prefixed attribute as expected.
A second call to setAttributeNS at a later time results in a new non-prefixed attribute. This is also the attribute accessed by getAttributeNS thereafter—the prefixed attribute appears to be inaccessible after the subsequent call. 
Subsequent calls to setAttributeNS modify the value of the non-prefixed attribute.

In Chrome and FF, repeat calls to setAttributeNS change the prefixed attribute's value.

I don't have the perspective to rate severity, but this behavior is _real_ unexpected on this end...

[Radar WebKit Bug Importer]

<rdar://problem/53712672>

[Chris Dumez]

I cannot reproduce on 10.15 beta. Either this has been fixed already or the test case is not 100% repro.

[Chris Dumez]

Oh, actually I can reproduce:
[Log] <g ex:attr="2"></g> (attachment.cgi, line 14)

[Log] <g ex:attr="2" attr="3"></g> (attachment.cgi, line 18)

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> Oh, actually I can reproduce:
> [Log] <g ex:attr="2"></g> (attachment.cgi, line 14)
> 
> [Log] <g ex:attr="2" attr="3"></g> (attachment.cgi, line 18)

Seems to be a serialization problem. The element really only has a single attribute with value=3. So at least, the internal representation looks correct. What we see in Web Inspector is incorrect though.

(In reply to Chris Dumez from comment #4)
> 
> Seems to be a serialization problem. The element really only has a single
> attribute with value=3. So at least, the internal representation looks
> correct. What we see in Web Inspector is incorrect though.

Interesting... 
I just modified the example to output g.outerHTML and... the attribute in the output never has a namespace prefix in Safari and STP... It always has a prefix in Chrome and FF...

I'm not sure what the spec is on that though...

[Chris Dumez]

XMLSerializer.serializeToString() returns the correct output:
"<g xmlns=\"http://www.w3.org/2000/svg\" ex:attr=\"3\" xmlns:ex=\"http://example.com\"/>"

g.outerHTML looks OK to:
"<g attr=\"3\"></g>"

I am not sure how Web Inspector is coming up with <g ex:attr="2" attr="3"></g>.

(In reply to Chris Dumez from comment #6)
> XMLSerializer.serializeToString() returns the correct output:
> "<g xmlns=\"http://www.w3.org/2000/svg\" ex:attr=\"3\"
> xmlns:ex=\"http://example.com\"/>"
> 
> g.outerHTML looks OK to:
> "<g attr=\"3\"></g>"
> 
> I am not sure how Web Inspector is coming up with <g ex:attr="2"
> attr="3"></g>.

Good to know about XMLSerializer behavior.
In Chrome and FF though g.outerHTML yields: <g ex:attr="1"></g>

(In reply to ian from comment #7)
> In Chrome and FF though g.outerHTML yields: <g ex:attr="1"></g>

(Point being that it contains the namespace prefix.)

[Chris Dumez]

(In reply to ian from comment #8)
> (In reply to ian from comment #7)
> > In Chrome and FF though g.outerHTML yields: <g ex:attr="1"></g>
> 
> (Point being that it contains the namespace prefix.)

Yes, this could be a bug too, although I would need to double check the spec.
I am more concerned about the serialization bug in Web Inspector at the moment though.

[Chris Dumez]

Created attachment 375174 [details]
WIP Patch

Here is a patch which fixes the issue. However, it would require a WebInspector test, which I am not really familiar about.

[Chris Dumez]

(In reply to ian from comment #8)
> (In reply to ian from comment #7)
> > In Chrome and FF though g.outerHTML yields: <g ex:attr="1"></g>
> 
> (Point being that it contains the namespace prefix.)

Tracking this in a separate bug: Bug 200283.

[Devin Rousso]

Created attachment 375184 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 375184 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375184&action=review

r=me

> LayoutTests/inspector/dom/attributeModified-expected.txt:9
> +Creating test node...
> +{
> +  "id": "with-namespace"
> +}

We should also have a test that nodes initially start with attributes that appropriately contain namespaces. Either here or in a different test.

> LayoutTests/inspector/dom/attributeModified.html:9
> +    node.id = id;

Here you could have a node that starts with a namespace attribute (including one that you don't modify):

    node.setAttributeNS("http://example.com", "ex:test-name", "test-value");

> LayoutTests/inspector/dom/attributeModified.html:74
> +    <p>Tests for the DOM.attributeModified event.</p>

I think normally we just drop this leading whitespace but I could be wrong.

[Chris Dumez]

Thanks Devin & Joe for taking care of this.

[Devin Rousso]

Created attachment 375218 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 375218 [details]
Patch

Clearing flags on attachment: 375218

Committed r248034: <https://trac.webkit.org/changeset/248034>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.