75700 - bad internal namespaceID when xmlns=""

Status: RESOLVED FIXED

(Core :: XML, defect, P2)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Excerpt from Namespaces in XML specification (http://www.w3.org/TR/REC-xml-names)
in section 5.2 :

   The default namespace can be set to the empty string. This has the same
   effect, within the scope of the declaration, of there being no default
   namespace.

It means that the namespace ID associated to an element carrying xmlns="" should
be kNameSpaceID_None (0x00000000) and that is not actually the case.

Run the test case attached to the current bug ; it is red because internally,
element testA has namespace ID 0x0000000c, as shown by VC6.
This test comes from my test suite associated to the CSS 3 Selectors Candidate
Recommandation.
The selector    |testA    has internally the correct 0x00000000 namespace ID
so testA is not selected by the rule.

I cannot at this point say if the problem comes from expat or from our
namespace manager.

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: xml test case showing bug

Comment 2

[Chris Petersen]

XMLNS related. Changing QA contact to [e-mail]

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is tricky. Because 0 is such a special value all over the code we cannot
just pre-register xmlns="" to kNameSPaceID_None (=0)like we do with the rest. If
we could bumb the namespace IDs by one (so that None would become 1 and so on)
we could do it. But since we are talking about a public interface here I don't
think that would be possible... But I'd love to be proved wrong.

Another solution is to treat xmlns="" specially in the namespace manager, but
that adds a little ugliness there and a minor slowdown of the code. We could
also register an internal xmlns="" namespace we'd need to translate when giving
values out. We could and should limit the hackiness in the namespace manager,
and not go and try to "fix" every client.

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix 1

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

In our current code base we can actually make do with a simple fix. The fix goes
to RegisternameSpace(). If we did not find that namespace and the URI was empty,
we report that the registered namespace ID was kXMLNameSpaceID_None.

We currently have three kinds of callers of RegisterNameSpace():

a) NameSpaceImpl constructor, which stores the ID into the impl.
b) NodeInfoManager explicitly will not call RegisterNameSpace() if URI is empty,
instead it automatically sets the ID to None for the node info. (We could fix
NIM to just call the function now.)
c) Callers with non-empty URI (XUL etc. namespace registration)

I think this fix is safe now and the only way it could break in the future is by
someone changing the namespace manager and/or namespace implementations.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Add to comment, let node info mgr call RegisterNameSpace()

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

harishd sez r=

(And don't need to initialize nsid in node info manager now, btw)

Comment 8

[Johnny Stenback (:jst)]

nsNameSpaceManager.cpp:

+    } else {
+      nsString* uri = new nsString(aURI);
+      gURIArray->AppendElement(uri); 
+      id = gURIArray->Count();  // id is index + 1
+      nsStringKey key(*uri);

No null check after new nsString? Fix that and sr=jst

Comment 9

[Johnny Stenback (:jst)]

nsNameSpaceManager.cpp:

+    } else {
+      nsString* uri = new nsString(aURI);
+      gURIArray->AppendElement(uri); 
+      id = gURIArray->Count();  // id is index + 1
+      nsStringKey key(*uri);

No null check after new nsString? Fix that and sr=jst

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed. I accidentally checked in a part of another bug as well, but it did not
matter (removal of unused include in nsXMLDocument.cpp).

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

nsBranch ok, simple fix.

Comment 12

[harishd]

Testcase, 30558, works fine.

Verified with July 5th trunk build.

Comment 13

[Nisheeth Ranjan]

This fix helps us perform better on the CSS3 tests.  It is simple, small, and
safe and has been on the trunk for more than a week.  Marking nsBranch+.

Comment 14

[Nisheeth Ranjan]

Reopening bug until the fix gets checked in on the branch.

Comment 15

[Nisheeth Ranjan]

This bug is lower priority and severity than bug 88354.  Since the PDT did not
accept the fix for bug 88354, I did not have any additional arguments to
convince them about this bug.  It is too late in the game for this fix to go
into the 6.1 branch.

Marking nsBranch-.

Comment 16

[Nisheeth Ranjan]

Resolving as fixed because this bug has been fixed on the trunk.