Bug 200230 - Web Inspector: Second call to setAttributeNS creates non-prefixed attribute
Summary: Web Inspector: Second call to setAttributeNS creates non-prefixed attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Safari 12
Hardware: Macintosh macOS 10.14
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-29 10:00 PDT by ian
Modified: 2019-07-31 01:53 PDT (History)
12 users (show)

See Also:


Attachments
Example. (471 bytes, text/html)
2019-07-29 10:00 PDT, ian
no flags Details
WIP Patch (1.41 KB, patch)
2019-07-30 14:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (6.98 KB, patch)
2019-07-30 14:58 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (7.21 KB, patch)
2019-07-31 01:11 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ian 2019-07-29 10:00:51 PDT
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...
Comment 1 Radar WebKit Bug Importer 2019-07-30 09:21:53 PDT
<rdar://problem/53712672>
Comment 2 Chris Dumez 2019-07-30 09:27:48 PDT
I cannot reproduce on 10.15 beta. Either this has been fixed already or the test case is not 100% repro.
Comment 3 Chris Dumez 2019-07-30 09:28:49 PDT
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)
Comment 4 Chris Dumez 2019-07-30 10:30:35 PDT
(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.
Comment 5 ian 2019-07-30 10:51:31 PDT
(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...
Comment 6 Chris Dumez 2019-07-30 10:53:50 PDT
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>.
Comment 7 ian 2019-07-30 11:02:39 PDT
(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>
Comment 8 ian 2019-07-30 11:03:29 PDT
(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.)
Comment 9 Chris Dumez 2019-07-30 11:55:25 PDT
(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.
Comment 10 Chris Dumez 2019-07-30 14:04:32 PDT
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.
Comment 11 Chris Dumez 2019-07-30 14:38:42 PDT
(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.
Comment 12 Devin Rousso 2019-07-30 14:58:41 PDT
Created attachment 375184 [details]
Patch
Comment 13 Joseph Pecoraro 2019-07-30 16:01:48 PDT
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.
Comment 14 Chris Dumez 2019-07-30 16:02:38 PDT
Thanks Devin & Joe for taking care of this.
Comment 15 Devin Rousso 2019-07-31 01:11:38 PDT
Created attachment 375218 [details]
Patch
Comment 16 WebKit Commit Bot 2019-07-31 01:53:45 PDT
Comment on attachment 375218 [details]
Patch

Clearing flags on attachment: 375218

Committed r248034: <https://trac.webkit.org/changeset/248034>
Comment 17 WebKit Commit Bot 2019-07-31 01:53:47 PDT
All reviewed patches have been landed.  Closing bug.