Closed
Bug 103225
Opened 23 years ago
Closed 20 years ago
[FIXr]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants
Categories
(Core :: XML, defect, P2)
Core
XML
Tracking
()
RESOLVED
FIXED
mozilla1.8beta1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
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.
Comment 1•23 years ago
|
||
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
Assignee | ||
Comment 3•23 years ago
|
||
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....
Updated•23 years ago
|
Target Milestone: --- → Future
Updated•22 years ago
|
QA Contact: petersen → rakeshmishra
Updated•21 years ago
|
QA Contact: rakeshmishra → ashishbhatt
Assignee | ||
Comment 4•21 years ago
|
||
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.
Comment 5•21 years ago
|
||
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?
Assignee | ||
Comment 6•21 years ago
|
||
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.
Comment 8•20 years ago
|
||
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
Comment 9•20 years ago
|
||
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
Assignee | ||
Comment 10•20 years ago
|
||
Huh? Why do we care about the element's namespace for HasAttr/GetAttr purposes,
exactly?
Comment 11•20 years ago
|
||
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.
Comment 12•20 years ago
|
||
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.
Assignee | ||
Comment 13•20 years ago
|
||
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.
Comment 14•20 years ago
|
||
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
Assignee | ||
Comment 16•20 years ago
|
||
Attachment #170320 -
Flags: superreview?(jst)
Attachment #170320 -
Flags: review?(jst)
Comment 17•20 years ago
|
||
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.
Comment 18•20 years ago
|
||
s/in the DOM/for the DOM/g
Assignee | ||
Comment 19•20 years ago
|
||
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.
Comment 22•20 years ago
|
||
(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.
Comment 23•20 years ago
|
||
(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 25•20 years ago
|
||
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 | ||
Updated•20 years ago
|
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
Assignee | ||
Updated•20 years ago
|
Summary: [FIX]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants → [FIXr]Fix nsGenericHTMLElement::GetNameSpaceID() and dependants
Assignee | ||
Comment 26•20 years ago
|
||
Fixed for 1.8b
Status: NEW → RESOLVED
Closed: 23 years ago → 20 years ago
Resolution: --- → FIXED
Comment 27•20 years ago
|
||
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.
Description
•