Extension of HTML elements has been broken. This is a regression and prevents XBL from being used to extend basic elements like <img>.

This was broken by jst in the fix for 132478. This change restricted the creation of HTML frames such that only HTML content nodes could make them.

The namespace checks here were intentional and (looking at the patch for the bug), it appears that other conversions to IsContentOfType were made, all of which break XML elements in other namespaces that extend both XUL and HTML elements.

As an example, I have been playing around with implementing xhtml2 using CSS/XBL, and this prevents me from simply extending existing HTML elements via XBL. As a side note it does look like you can still extend XUL at least, since the ConstructXULFrame method was not changed, although the root XUL elements will not extend correctly now.

Oops, the bug was 134278. I had the digits inverted.

This patch adds support for XHTML2 elements, and makes sure they get treated like XHTML1 elements. I also implemented the ability to jump to anchors in XHTML2 (which uses id rather than name).

Don't you need to put the uri in gURIArray? > + gURIToIDTable->Put(&mathmlKey, NS_INT32_TO_PTR(kNameSpaceID_XHTML2)); Wrong key? > // XXX This is incorrect!!!!!!!!!!!!!!!! > + // XXX It could be XHTML or XHTML2 or... etc. etc. The reason GetNamespaceID is hacked like this is because it's used in style resolution. This means that if you want XHTML2 elements to match rules in the XHTML2 namespace this function needs to return the right namespace id.... Perhaps this function should check the nodeinfo namespace id and if that's kNameSpaceID_None (I think that's the right thing to check for) it should fall back on kNameSpaceID_HTML? > + cssParser->SetCaseSensitive(mNodeInfo->NamespaceEquals(kNameSpaceID_XHTML) || > + mNodeInfo->NamespaceEquals(kNameSpaceID_XHTML2)); Here, also, it may be better to switch to SetCaseSensitive(!mNodeInfo->NamespaceEquals(kNameSpaceID_None)), assuming that does the right thing. > + if (mNodeInfo->NamespaceEquals(kNameSpaceID_XHTML) || > + mNodeInfo->NamespaceEquals(kNameSpaceID_XHTML2)) { And here...I'll stop commenting on this; we should just decide what the right approach is for callsites like these, where we know we have HTML and just don't know whether it's tag-soup or not. Scrolling to id should work in XHTML and HTML4.0, not just in XHTML2.... and it applies to all elements, not just <a>. I would hope we have code that already does that (checks for <a name="foo"> first, then just does document.getElementById("foo") if we are going to #foo). If we do not, we should. See http://www.w3.org/TR/html401/struct/links.html#h-12.2.3

Duh. Missed the id checking code at the top of the function. Verifying with my XHTML2 sample now...

I tried returning kNameSpaceID_None in the HTML element function, but that causes the UA sheet not to match (which surprised me). So I've left that alone. I fixed the other problem though (of assuming None).

Bah. ROFL. Ignore the changes to nsPresShell.cpp in that patch. Just look at the rest.

Comment on attachment 104084 [details] [diff] [review] Better patch. This one was actually compiled and tested (gasp!). nsGenericHTMLElement::GetNameSpaceID(PRInt32& aID) const { - // XXX - // XXX This is incorrect!!!!!!!!!!!!!!!! - // XXX - aID = kNameSpaceID_XHTML; + aID = kNameSpaceID_XHTML; // XXX Return this for XHTML and XHTML2, so that UA styles can apply to both. -dwh return NS_OK; } Do *NOT* remove that XXX comment, that method is broken, the method should return what you get from mNodeInfo->GetNameSpaceID(), but as you realized, mozilla kinda falls appart if you do that. That however is a bug in the style system, not a problem with this code...

Oh, and in stead of checking if an element's namespace ID is kNameSpaceID_XHTML2? use nsIContent::IsContentOfType(nsIContent::eHTML) when possible.

My point was that the style system uses GetNamespaceID() to get the namespace... so if you have a sheet in the XHTML namespace it will match (with that patch), but one in the XHTML2 namespace will not.... So how exactly are you applying the UA rules to XHTML2 elements (eg the XBL bindings)? By putting them in a sheet in the XHTML namespace? That magic namespace directive is sounding mighty nice.... that would move this CSS-specific hack over into the CSS part of the style system...

Correct, my new sheets are in the XHTML namespace instead of the XHTML2 namespace. I do think eventually we should use a magic directive in the CSS files, but I'm content to use the wrong namespace in my UA sheet for now (although you're right that namespaced author sheets in the XHTML2 namespace will not apply correctly). I'd be happy to do this before landing if the reviewers feel like it's required. I'd need pointers to the CSS code though.

IMHO namespaced author sheets in the XHTML2 namespace not applying correctly is a big deal.

Ok, this patch fixes the bug with GetNameSpaceID in HTML element and just returns the node info's namespace ID. I patched the style system to understand kNameSpaceID_None, and I implemented a new magic namespace called http://www.mozilla.org/xhtml-generic that matches HTML, XHTML and XHTML2. This patch also fixes ConstructHTMLFrame to also check the XHTML and XHTML2 namespaces in the event that the content node is not HTML but extends an HTML namespace (which happens when using XBL). I have also adjusted all stylesheets (including the prefs sheet) to use the new magic namespace where appropriate. Ready for comments/feedback (or r/sr if you like it that much right out of the box). ;)

This seems wrong, both before and after your patch: Index: content/html/style/src/nsDOMCSSAttrDeclaration.cpp // look up our namespace. If we're XHTML, we need to be case-sensitive // Otherwise, we should not be - (*aCSSParser)->SetCaseSensitive(nodeInfo->NamespaceEquals(kNameSpaceID_XHTML)); + (*aCSSParser)->SetCaseSensitive(nodeInfo->NamespaceEquals(kNameSpaceID_XHTML) || + nodeInfo->NamespaceEquals(kNameSpaceID_XHTML2));

Shouldn't you be checking that the content is HTML content in the kNameSpaceID_None cases in selector-matching? We don't want to match non-namespaced XML content with xhtml-namespaced CSS... Hixie, the code you quote is correct because that's all working with HTML elements only, so the only question there is just whether it's SGML-based HTML or XHTML.

I really don't like this bogus generic namespace. If the DOM WG really insists on being so pedantically correct rather than having a useful API, then the CSS WG probably needs to reconsider the interaction of selectors and namespaces. What does this change imply for our compliance to the css3-namespaces draft and the parts of it in css3-selectors?

(Actually, though, I'm having trouble finding the part of http://www.w3.org/TR/DOM-Level-2-Core/ or http://www.w3.org/TR/DOM-Level-2-HTML/ that says how DOM representations of SGML-based documents must handle namespace-related methods and properties. What says so?)

Another idea (although even more involved) is to enable @namespace to support multiple URIs (i.e., allow the establishing of multiple default namespaces), and/or the defining of prefixes that match multiple namespaces.

Ok, ignoring the controversial XHTML2 portion of the patch, here is the patch to just fix the XBL regression (without introducing anything regarding XHTML2).

dbaron: I don't think this patch changes our compliance in any measurable way (except if a test actually uses the magic namespace, which would be like a test using a -vnd-foo property). All it does is make the @namespace rule for a particular namespace match three namespaces rather than one. The DOM is not affected in any way. @namespace rules for XHTML namespaces will continue to match in the same way. Note that we actually already have this hack in a way, as before this patch the XHTML1 namespace also matched HTML4 content, which is incorrect. This patch fixes this as well.

Comment on attachment 104158 [details] [diff] [review] Patch that just fixes the XBL problem. r/sr=bzbarsky

Boarf. DavidH, your problem is only caused by badly chosen namespace URIs. xhtml2 namespace should be http://www.w3.org/html/xhtml2/2002/06, not http://www.w3.org/2002/06/xhtml2. Then you could just use @namespace html url("http://www.w3.org/html/"); to select all namespaces "under" this URI, including html, xhtml1, xhtml2. DavidH, your proposal of multiple URIs per @namespace rule could perfectly raised to the WG. Ian, I think you are wrong, this patch changes our conformance to Selectors CR. Perhaps not from a test suite POV, but one unique @namespace cannot match both xhtml1 and xhtml2 languages.

The problem, of course, is that xhtml1 and xhtml2 are not really distinct languages... I'm not sure I follow what a different namespace for xhtml2 would enable us to do, but if it would help, is XHTML2 so far advanced that changing its namespace would be impossible at this point?

I'm fairly certain we don't treat the @namespace directive as a prefix match, but probably require an identical match. Even if we did, that is a more expensive computation that must be performed (testing for prefix matching). I suppose there are clever things we could do for known namespaces. As bz said, though, is the xhtml2 namespace likely to change? The chosen namespace may be deliberate, since they claim not to be backwards compatible with xhtml1 (and thus may want to intentionally set themselves apart). Despite this claim, many of the tags are absolutely identical and it makes no sense for us to duplicate all our UA rules. :)

> Then you could just use @namespace html url("http://www.w3.org/html/"); > to select all namespaces "under" this URI, including html, xhtml1, xhtml2. This is incorrect; namespace URIs are by definition opaque and prefix matching is therefore out of the question. > Ian, I think you are wrong, this patch changes our conformance to Selectors CR It's an extension. Extensions are legitimate, and are not considered to break conformance, if they are carefully insulated from standard usage, as in this case. With properties, our extensions are denoted by a '-moz-' prefix, with namespaces, our extensions are denoted by namespaces in the mozilla.arg space.

Can someone r= the XBL-only patch? :)

> namespace URIs are by definition opaque and prefix matching > is therefore out of the question. Absolutely. That's my point : it's a design mistake. It's a namespace URI and not a namespace URN. It uses all the bad from URNs and not the good from URIs. Anyway.

Comment on attachment 104158 [details] [diff] [review] Patch that just fixes the XBL problem. r/sr=jst

hyatt, are you planning to check in that reviewed patch sometime?

Comment on attachment 104158 [details] [diff] [review] Patch that just fixes the XBL problem. I just checked this patch in.

Wouahhhh. I often follow checkin activities, but this one, with more 13 months between review and checkin is I think a candidate for a record!