Misleading documentation of nsIContent::GetAttribute()

VERIFIED FIXED

Status

()

Core
Layout
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: rbs, Assigned: vidur (gone))

Tracking

Trunk
x86
Windows 98
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

18 years ago
The return value of nsIContent::GetAttribute() is documented as
follows:

  /**
   * Get the current value of the attribute. This returns a form that is
   * suitable for passing back into setAttribute.
   *
   * <UL>
   *
   * <LI>If the attribute is not set and has no default value, return
   * NS_CONTENT_ATTR_NOT_THERE.
   *
   * <LI>If the attribute exists, but has no value, return
   * NS_CONTENT_ATTR_NO_VALUE.
   *
   * <LI>If the attribute has a value, empty or otherwise, set ret to
   * be the value, and return NS_CONTENT_ATTR_HAS_VALUE (== NS_OK).
   *
   * </UL>
   */
  NS_IMETHOD GetAttribute(PRInt32 aNameSpaceID, nsIAtom* aName, 
                          nsString& aResult) const = 0;

When reading the above documentation, the frame of mind is that 
it caters for the following cases (in no particular order).

(1) <tag attribute="value">
(2) <tag attribute="">
(3) <tag attribute>  <!-- eg. as in <td nowrap> -->
(4) <tag>

Trying to guess which is which from the documentation...
Af first, one would think that 

case 3 (no value) corresponds to:
   * <LI>If the attribute exists, but has no value, return
   * NS_CONTENT_ATTR_NO_VALUE.

case 1 (non-empty value) and 2 (empty value) correspond to:
   * <LI>If the attribute has a value, empty or otherwise, set ret to
   * be the value, and return NS_CONTENT_ATTR_HAS_VALUE (== NS_OK).

case 4 (attribute not specified) corresponds to:
   * <LI>If the attribute is not set and has no default value, return
   * NS_CONTENT_ATTR_NOT_THERE.

But as plausible as this guess may be, it is wrong, and has led to
MathML bug 31642. 

After checking with some printf(), it appears that:

<tag attribute="value"> means NS_CONTENT_ATTR_HAS_VALUE
<tag attribute="">      means NS_CONTENT_ATTR_NO_VALUE
<tag attribute>         ??
<tag>                   means NS_CONTENT_ATTR_NOT_THERE

It is somewhat confusing, especially given the mention of 
"empty or otherwise" in the documentation.
Well, 
   <tag attribute>
...is invalid XML and in SGML (i.e., HTML) actually stands for:
   <tag real-attribute=attribute>
e.g.
   <table border>
...actually means
   <table frame="border">
...(not border="1") and
   <td nowrap>
...actually means
   <td nowrap="nowrap">

I don't know if we are internally doing this correctly, but that would explain
the "??" next to "<tag attribute>". What did GetAttribute() actually return for
that case? Did the XML parser even accept it?
(Reporter)

Comment 2

18 years ago
No, the XML parser didn't accept it. Also I don't know what the results would
be in HTML. But my guess is that someone reading the documentation would
understand "empty or otherwise" as I explained above, i.e., as attribute="",
or attribute="value". And this is not what the code is doing. On the contrary,
NS_CONTENT_ATTR_HAS_VALUE is returned only if the value is non-empty,
therefore it would be clearer to remove "empty or otherwise", and simply
replace it with "non-empty value".
Keywords: donttest
(Assignee)

Comment 3

18 years ago
True enough. I fixed the comment.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 4

18 years ago
Marking VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.