Closed Bug 31736 Opened 25 years ago Closed 24 years ago

Misleading documentation of nsIContent::GetAttribute()

Categories

(Core :: Layout, defect, P3)

x86
Windows 98
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: rbs, Assigned: vidur)

References

()

Details

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?
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
True enough. I fixed the comment.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Marking VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.