Closed Bug 103225 Opened 23 years ago Closed 20 years ago

[FIXr]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants

Categories

(Core :: XML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta1

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Currently XHTML and HTML are keyed to the same thing in the namespace manager. This means that setting case-sensitivity of CSS parsing based on the namespace of a node is impossible -- the node would always seem as if it were in both XHTML and HTML namespaces.
Actually this is detectable, an elements nsINodeInfo knows the difference, but nsIContent::GetNameSpaceID() does not. I believe heikki has a fix in another bug for the broken GetNameSpaceID() implementation so there's no need for this bug. Marking INVALID (even if I was the one telling bz to file this bug).
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → INVALID
No, I don't have the fix. We know what the issue is, and I think it would be better to take care of it in here. The element's node info knows the difference between HTML and XHTML, but GetNameSpaceID() always gives XHTML namespace. It needs to be fixed, but additionally something needs to be done about the UA stylesheets or stylesheet handling. See jst's and hyatt's comments on 2001-10-09 in bug 41983. Changing Summary to reflect the bug. Boris, wanna have this? ;)
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Summary: No way to tell whether an HTML element is in XHTML or HTML namespace → Fix nsGenericHTMLElement::GetNameSpaceID() and dependants
Heikki, if I took this I would not get to it for months (working on style stuff and non-mozilla life). I mostly filed it so we would have a bug on file for the problem....
Target Milestone: --- → Future
QA Contact: petersen → rakeshmishra
QA Contact: rakeshmishra → ashishbhatt
So we have the following callers of GetNamespaceID in places where we could have non-XHTML HTML: 1) Frame constructor (this would be unaffected by fixing it, since it does IsContentOfType checks) 2) XBL ResolveTag stuff (again, should be fine, since this is just looking up a namespace id for the frame constructor) 3) Accessibility (no idea here. Aaronl?) 4) The style matching code, which can simply change: aContent->GetNamespaceID(&mNamespaceID); into (20 lines lower, if we want to make use of the existing IsContentOfType() check): if (aContent->IsContentOfType(nsIContent::eHTML)) mNamespaceID = kNameSpaceID_XHTML; else aContent->GetNamespaceID(&mNamespaceID); So assuming we're OK with that change to style code (David?) and that accessibility doesn't have a problem, we should just fix this.
As far as the accessibility code is concerned it doesn't matter whether we're in HTML or XHTML. That would change if we added support for XHTML 2.0, which would have much bigger differences. However, should I be concerned about that now? Do I ever need to worry about someone missing XHTML 1.1 an XHTML 2.0 in the same document?
If we ever implement XHTML2.0, then yes. But they'd have different namespace IDs, so you'd be OK. This bug is just about non-XHTML HTML elements.
Aaron: I think you'd need to be concerned even without xhtml2. Fixing nsGenericHTMLElement::GetNameSpaceID() means making it return kNameSpaceID_None for HTML elements while still returning kNameSpaceID_XHTML for XHTML elements. So you might need to switch to using IsContentOfType(nsIContent::eHTML) or doing something like the snippet in comment 4.
Hmm, I fixed most of these once, but new ones crept into the accessibility code :( Jonas, how about something like: if (content->HasAttr(content->GetNamespaceID(&mNamespaceID), nsAccessibilityAtoms::alt)) { } However, to me it seems like it would be better to change HasAttr, GetAttr and friends to take something like kNameSpaceID_SameAsElement
Hmm, I fixed most of these once, but new ones crept into the accessibility code :( Jonas, how about something like: if (content->HasAttr(content->GetNamespaceID(&mNamespaceID), nsAccessibilityAtoms::alt)) { } However, to me it seems like it would be better to change HasAttr, GetAttr and friends to take something like kNameSpaceID_SameAsElement
Huh? Why do we care about the element's namespace for HasAttr/GetAttr purposes, exactly?
Aaronl, it's really really rare for attributes to ever be in a namespace, attributes don't inherit their namespace from the element or anything like that. Only attributes with an explicit prefix are in a namespace.
Okay, I'm not sure what Jonas was saying I need to change. When do I need to be careful of xhtml/html differences? Can I just keep using kNameSpaceID_None when using GetAttr and HasAttr in the accessibility module? For example I just want to get the alt attribute for an img element.
Yes, you could. The one place in accessibility that calls GetNameSpaceID is nsAccessNodeWrap::get_nodeInfo. It returns the namespace ID to the caller; I don't know what callers do with it, so I'm not sure what the impact of it suddenly returning kNamespaceID_None for non-XHTML HTML would be.
No one is using that yet, afaik.
As bz and jst said, this does not affect attributes. In almost all cases you want to use the namespace kNameSpaceID_None when getting and setting attributes. The exception to this is attributes that use a namespace prefix, like xbl:inherit and xlink:href. The bug only affects the namespace of the element itself, and only when calling GetNameSpaceID(). Also note that it's about changing the namespace *from* kNameSpaceID_XHTML *to* kNameSpaceID_None
Attached patch Fix. (deleted) — Splinter Review
Attachment #170320 - Flags: superreview?(jst)
Attachment #170320 - Flags: review?(jst)
As you probably all know, I'm against this, and what we do in the DOM. Though mainly what we do in the DOM rather than this, since it makes it much harder for DOM script authors to write scripts that work on both (1) old-fashioned HTML and (2) XHTML within multi-namespace documents. And I don't think any spec requires that we do what we do in the DOM.
s/in the DOM/for the DOM/g
Actually, I did not know that... If you're not ok with this patch, please feel free to mark r- on it. I won't mind. ;)
dbaron: what is it that you don't like? The DOM is unaffected by this patch. It only changes an inconsitency for internal callers (currently some callers are forced to use node->GetNodeInfo()->NamespaceID() rather then node->GetNameSpaceID() )
Ah, sorry, reread your comment and I understand what you mean. While I agree that returning the xhtml namespace for html would ease creating cross xhtml-html applications (just see my patches in bug 1995), I still think we should check in this patch. First of all the current state is confusing and inconsistent, where the nsIDOMNode methods and the nsINodeInfo returns one namespace, while nsIContent::GetNameSpaceID returns another. Second, if we do decide on making html return the xhtml namespace we still want this patch since in that case the nodeinfo should have an xhtml namespaceID and the method in nsGenericElement would work just fine.
(In reply to comment #21) > Second, if we do decide on making html return the xhtml namespace we still want > this patch since in that case the nodeinfo should have an xhtml namespaceID and > the method in nsGenericElement would work just fine. Right, so I guess this is fine with me.
(except that it would expose yet more callers to a change if we decide to change back)
True, but it would be a pretty big change anyway since we have quite a few callers that use the namespaceID of the nodeinfo. I think we're in one of those situations where there just aren't any good solutions, just more or less bad ones.
Comment on attachment 170320 [details] [diff] [review] Fix. Very nice! r+sr=jst
Attachment #170320 - Flags: superreview?(jst)
Attachment #170320 - Flags: superreview+
Attachment #170320 - Flags: review?(jst)
Attachment #170320 - Flags: review+
Assignee: hjtoi-bugzilla → bzbarsky
Status: REOPENED → NEW
Priority: -- → P2
Summary: Fix nsGenericHTMLElement::GetNameSpaceID() and dependants → [FIX]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants
Target Milestone: Future → mozilla1.8beta
Summary: [FIX]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants → [FIXr]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants
Blocks: 277297
Fixed for 1.8b
Status: NEW → RESOLVED
Closed: 23 years ago20 years ago
Resolution: --- → FIXED
I try to keep my head out of layout/* :), but this patch might have created bug 279027. Our XForms XTF elements are suddenly not in the XForms namespace anymore?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: