Closed Bug 277779 Opened 20 years ago Closed 20 years ago

e4x: calling setNamespace on element with already existing default namespace shows two xmlns declarations in toXMLString which is not well-formed

Categories

(Core :: JavaScript Engine, defect, P2)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Windows XP
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.8beta1

People

(Reporter: martin.honnen, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(2 files)

proposed fix
4.59 KB, patch
shaver
: review+
Details | Diff | Splinter Review
e4x/Regress/regress-277779.js
2.55 KB, text/plain
Details 
The test case below uses setNamespace to change the namespace of an element that
has been statically set to be in a default namespace (e.g. has an xmls="someURL"
'attribute').
When calling toXMLString then on the object Spidermonkey shows two xmlns
'attributes' which is not well-formed and thus a bug.

Test case:

function testNamespaces (xml) {
  var result = '';
  result += 'Checking ' + xml.toXMLString() + ' : \r\n';
  result += '.namespace(): ' + xml.namespace() + '\r\n';
  result += '.namespaceDeclarations().length: ' +
xml.namespaceDeclarations().length + '\r\n';
  result += '.namespaceDeclarations(): ' + xml.namespaceDeclarations() + '\r\n';
  return result;
}

var xhtml2NS = new Namespace('http://www.w3.org/2002/06/xhtml2');
var xhtml = <html xmlns="http://www.w3.org/1999/xhtml" />;
xhtml.setNamespace(xhtml2NS);

var result = testNamespaces(xhtml);

if (typeof alert == 'function') {
  alert(result);
}
else if (typeof print == 'function') {
  print(result);
}

Result with Spidermonkey:

Checking <html xmlns="http://www.w3.org/1999/xhtml"
xmlns="http://www.w3.org/2002/06/xhtml2"/> : 
.namespace(): http://www.w3.org/2002/06/xhtml2
.namespaceDeclarations().length: 1
.namespaceDeclarations(): http://www.w3.org/1999/xhtml

Result with Rhino:

Checking <xht:html xmlns="http://www.w3.org/1999/xhtml"
xmlns:xht="http://www.w3.org/2002/06/xhtml2"/> :
.namespace(): http://www.w3.org/2002/06/xhtml2
.namespaceDeclarations().length: 2
.namespaceDeclarations():
http://www.w3.org/1999/xhtml,http://www.w3.org/2002/06/xhtml2
See bug 246441 comment 77.  I have a fix for SpiderMonkey that I'll attach here,
but it counts on us marking bug 277774 INVALID per ECMA-357.

/be
Status: NEW → ASSIGNED
Attached patch proposed fix — — Splinter Review 
1.  Move static namespace_match up to use in XMLToXMLString.  Note that it
matches prefixes only when called with a namespace with a non-null prefix, as
is done in 2 here:

2.  When overriding the default namespace for the tag's name in XMLToXMLString,
be sure to delete any explicit default namespace from the decls local XMLArray,
just before appending the new default namespace derived from the tag name.

How, you might reasonably ask, could the two differ?  Only if you follow the
spec for XML.prototype.setNamespace (13.4.4.36) and update x.[[Name]] to use a
namespace that does not match the declared default.  Is it a spec bug that E4X
allows this incoherence?  The alternative does violence to the notion of
"declared namespaces" underlying XML.prototype.namespaceDeclarations().

/be
Assignee: general → brendan
Attachment #170876 - Flags: review?(shaver)
Keywords: js1.5
Priority: -- → P2
Target Milestone: --- → mozilla1.8beta
Comment on attachment 170876 [details] [diff] [review]
proposed fix

r=shaver.  This ECMA-357 namespace stuff seems like a bit of a mess, from this
vantage point.
Attachment #170876 - Flags: review?(shaver) → review+
Fixed.  I'll bring up the erratum this spawned with the ECMA TG1 folks tomorrow.

/be
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Verified fixed with Mozilla 1.8b (Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.8b) Gecko/20050122)
Status: RESOLVED → VERIFIED
Martin, with your permission this will be included in the javascript test
library.
(In reply to comment #6)
> Created an attachment (id=174989) [edit]
> e4x/Regress/regress-277779.js

Note that in the test case I posted there are two issues, one being the result
of xml.namespace() should change to reflect the newly set namespace, and the
other being the result of xml.namespaceDeclarations().length. If I understand
Brendan correctly then Spidermonkey has been fixed to change the default
namespace and make sure that not two xmlns="URI" attributes are serialized. 
So in your test case

expect = 'http://www.w3.org/2002/06/xhtml2';
actual = xml.namespace().toString();
TEST(1, expect, actual);

is fine.

But the result of xml.namespaceDeclarations().length is actually a different
issue, namely bug https://bugzilla.mozilla.org/show_bug.cgi?id=277774, and
according to the statements there the length should not change so a bug on Rhino
has been filed for that.

Thus in your test case I think you need

expect = 1;
actual = xml.namespaceDeclarations().length;
TEST(2, expect, actual);

and

expect = 'http://www.w3.org/1999/xhtml';
actual = xml.namespaceDeclarations().toString();
TEST(3, expect, actual);

but perhaps it makes more sense to make a new test case for that issue, I think
Igor has already done so: https://bugzilla.mozilla.org/show_bug.cgi?id=278112#c1
2 & 3 removed, thanks. I'll mark you as "Ok" on the contributions.
e4x/Regress/regress-277779.js checked in.
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: