Skip to content

Commit

Permalink
Merge "XMLSerializer: Keep redundant but harmless default namespace d…
Browse files Browse the repository at this point in the history
…eclarations" to M74 branch

Since crrev.com/632142, XMLSerializer has dropped redundant xmlns="..."
declarations. It matches to IE, Edge, and DOM P&S standard.  This CL
reverts the behavior to unbreak existing applications.  The restored
behavior matches to Firefox and Safari.

* MarkupAccumulator::AppendElement():
  Even if ignore_namespace_definition_attribute_ is set, we drop a
  xmlns="..." only if its value is inconsistent with element's
  namespace.

* MarkupAccumulator::AppendStartTagOpen():
  if local_default_namespace is "" and ns is null, do not emit xmlns="".
  This avoids to serialize xmlns="" twice.

Bug: w3c/DOM-Parsing#47
Bug: 940204
Change-Id: I2978ddc9a3f9511d227a9a1b902f1811ac1c3c07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1516124
Reviewed-by: Yoshifumi Inoue <[email protected]>
Commit-Queue: Kent Tamura <[email protected]>
Auto-Submit: Kent Tamura <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#639873}(cherry picked from commit 31f0236 [formerly 6208aa43211c09758870c44f7dd7c737e2cc3110])
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524508
Reviewed-by: Kent Tamura <[email protected]>
Cr-Commit-Position: refs/branch-heads/3729@{#147}
Cr-Branched-From: 1926073 [formerly d4a8972e30b604f090aeda5dfff68386ae656267]-refs/heads/master@{#638880}
Former-commit-id: f75d3ff1174b12dce871d1f942ed8798296971c9
  • Loading branch information
tkent-google committed Mar 15, 2019
1 parent c800b2b commit 986e5b7
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,12 @@ AtomicString MarkupAccumulator::AppendElement(const Element& element) {
for (const auto& attribute : element.Attributes()) {
if (data.ignore_namespace_definition_attribute_ &&
attribute.NamespaceURI() == xmlns_names::kNamespaceURI &&
attribute.Prefix().IsEmpty())
continue;
attribute.Prefix().IsEmpty()) {
// Drop xmlns= only if it's inconsistent with element's namespace.
// https://github.com/w3c/DOM-Parsing/issues/47
if (!EqualIgnoringNullity(attribute.Value(), element.namespaceURI()))
continue;
}
if (!ShouldIgnoreAttribute(element, attribute))
AppendAttribute(element, attribute);
}
Expand Down Expand Up @@ -315,7 +319,8 @@ MarkupAccumulator::AppendStartTagOpen(const Element& element) {

// 12.6. Otherwise, if local default namespace is null, or local default
// namespace is not null and its value is not equal to ns, then:
if (local_default_namespace.IsNull() || local_default_namespace != ns) {
if (local_default_namespace.IsNull() ||
!EqualIgnoringNullity(local_default_namespace, ns)) {
// 12.6.1. Set the ignore namespace definition attribute flag to true.
data.ignore_namespace_definition_attribute_ = true;
// 12.6.3. Let the value of inherited ns be ns.
Expand All @@ -331,7 +336,7 @@ MarkupAccumulator::AppendStartTagOpen(const Element& element) {
// 12.7. Otherwise, the node has a local default namespace that matches
// ns. Append to qualified name the value of node's localName, let the value
// of inherited ns be ns, and append the value of qualified name to markup.
DCHECK_EQ(local_default_namespace, ns);
DCHECK(EqualIgnoringNullity(local_default_namespace, ns));
namespace_context.SetContextNamespace(ns);
formatter_.AppendStartTagOpen(markup_, element);
return data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ This is a testharness.js-based test.
PASS check XMLSerializer.serializeToString method could parsing xmldoc to string
PASS Check if the default namespace is correctly reset.
PASS Check if there is no redundant empty namespace declaration.
FAIL Check if redundant xmlns="..." is dropped. assert_equals: expected "<root><child/></root>" but got "<root><child xmlns=\"\"/></root>"
PASS Check if inconsistent xmlns="..." is dropped.
PASS Check if an attribute with namespace and no prefix is serialized with the nearest-declared prefix
FAIL Check if an attribute with namespace and no prefix is serialized with the nearest-declared prefix even if the prefix is assigned to another namespace. assert_equals: expected "<el1 xmlns:p=\"u1\" xmlns:q=\"u1\"><el2 xmlns:q=\"u2\" q:name=\"v\"/></el1>" but got "<el1 xmlns:p=\"u1\" xmlns:q=\"u1\"><el2 xmlns:q=\"u2\" p:name=\"v\"/></el1>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ <h1>domparsing_XMLSerializer_serializeToString</h1>
assert_equals(serialize(root), '<root xmlns="urn:bar"><outer xmlns=""><inner>value1</inner></outer></root>');
}, 'Check if there is no redundant empty namespace declaration.');

test(function() {
assert_equals(serialize(parse('<root><child xmlns=""/></root>')),
'<root><child/></root>');
assert_equals(serialize(parse('<root xmlns=""><child xmlns=""/></root>')),
'<root><child/></root>');
assert_equals(serialize(parse('<root xmlns="u1"><child xmlns="u1"/></root>')),
'<root xmlns="u1"><child/></root>');
}, 'Check if redundant xmlns="..." is dropped.');

test(function() {
const root = parse('<root xmlns="uri1"/>');
const child = root.ownerDocument.createElement('child');
Expand Down Expand Up @@ -145,8 +154,6 @@ <h1>domparsing_XMLSerializer_serializeToString</h1>

test(function() {
assert_equals(serialize(parse('<root><child/></root>')), '<root><child/></root>');
assert_equals(serialize(parse('<root><child xmlns=""/></root>')), '<root><child/></root>');
assert_equals(serialize(parse('<root xmlns="u1"><child xmlns="u1"/></root>')), '<root xmlns="u1"><child/></root>');
assert_equals(serialize(parse('<root xmlns="u1"><p:child xmlns:p="u1"/></root>')), '<root xmlns="u1"><child xmlns:p="u1"/></root>');
}, 'Check if start tag serialization drops element prefix if the namespace is same as inherited default namespace.');

Expand Down

0 comments on commit 986e5b7

Please sign in to comment.