Open Bug 504488 Opened 11 years ago Updated 2 years ago

nsXULElement::AfterSetAttr doesn't always guard its use of aValue

Categories

(Core :: XUL, defect)

defect
Not set

Tracking

()

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file)

in most places nsXULElement::AfterSetAttr checks aValue before using it....
Attached patch patchSplinter Review
mq diff's have limited context, here's the critical bit:
+            document && document->GetRootContent() == this &&
+            aValue) {
 
             nscolor color = NS_RGBA(0, 0, 0, 0);
             nsAttrValue attrValue;
             attrValue.ParseColor(*aValue, document);
Attachment #388858 - Flags: review?(neil)
Comment on attachment 388858 [details] [diff] [review]
patch

Normal elements call AfterSetAttr with a null aValue from UnsetAttr. While they currently do not, it is possible that at some point that it would be desirable for XUL elements to call AfterSetAttr from UnsetAttr. I therefore consider that the code should either be written to mirror the work UnsetAttr does, or that it should not null-check aValue at all, however which it should be is not my call.
Attachment #388858 - Flags: review?(neil)
You need to log in before you can comment on or make changes to this bug.