Closed Bug 230877 Opened 21 years ago Closed 21 years ago

scrollbar thumb does not draw if MOZ_XUL not defined

Categories

(Core :: XML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bryner, Assigned: bryner)

Details

Attachments

(1 file, 1 obsolete file)

Scrollbar thumbs don't show up if MOZ_XUL is not #defined.  This is a
side-effect of IsContentOfType(eXUL) returning FALSE, because we use
nsXMLElement for XUL elements with MOZ_XUL undefined (i.e. we don't use any of
the XUL cache or prototype stuff).  This affects the scrollbar by causing
whitespace not to be stripped by the XBL content sink, causing the slider to get
an extra text node child, and therefore not being able to locate its thumb in
the expected place.
Attached patch patch (obsolete) — Splinter Review
Allow XMLElement to claim that the content is of type XUL if that is the
namespace prefix.
Comment on attachment 139017 [details] [diff] [review]
patch

jst, can you r+sr?  (I'm open to suggestions if you'd rather I #ifdef this
differently, or not at all)
Attachment #139017 - Flags: superreview?(jst)
Attachment #139017 - Flags: review?(jst)
Comment on attachment 139017 [details] [diff] [review]
patch

+#ifdef MOZ_XUL
+PRBool
+nsXMLElement::IsContentOfType(PRUint32 aFlags) const
+{
+  PRBool isXUL = mNodeInfo->GetPrefixAtom() == nsLayoutAtoms::xulNameSpace;
+  return !(aFlags & ~(eELEMENT | (isXUL ? eXUL : 0)));
+}
+#endif

This would make any element that happened to use a prefix named 'xul' be
treated as if it was of type eXUL, and that's not what we want (anyone is free
to use 'xul' as an XML prefix), even in this odd case. What you should do is to
check that mNodeInfo->NamespaceID() == kNameSpaceID_XUL, then it would do the
right thing in all cases. (And there's no need for the new static atom).
Attachment #139017 - Flags: superreview?(jst)
Attachment #139017 - Flags: superreview-
Attachment #139017 - Flags: review?(jst)
Attachment #139017 - Flags: review-
Attached patch patchSplinter Review
tweak xbl whitespace check to look for XUL namespace, not IsContentOfType. 
This lets it work even for XMLElement-implemented XUL elements.
Attachment #139017 - Attachment is obsolete: true
Comment on attachment 139052 [details] [diff] [review]
patch

Yeah, that works, and doesn't break the assumption that eXUL elements really
are XUL elements, i.e. can be QI'ed etc to nsIXUL... (as we discussed).

r+sr=jst
Attachment #139052 - Flags: superreview+
Attachment #139052 - Flags: review+
checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: