Closed
Bug 373589
Opened 18 years ago
Closed 18 years ago
Crash [@ nsPresContext::DefaultLinkColor] trying to get link color for <body> that has been removed from the document
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Unassigned)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(3 files, 4 obsolete files)
|
189 bytes,
text/html
|
Details | |
|
3.13 KB,
patch
|
Details | Diff | Splinter Review | |
|
2.11 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Regression range: 2007-03-09 nightly to 2007-03-10 nightly. I suspect the change in bug 372971, which converted nsHTMLBodyElement::GetLink and friends from macro-uses to functions.
Exception: EXC_BAD_ACCESS (0x0001)
Codes: KERN_PROTECTION_FAILURE (0x0002) at 0x000000a8
Thread 0 Crashed:
0 libgklayout.dylib 0x19caa659 nsPresContext::DefaultLinkColor() const + 9 (nsIDocument.h:335)
1 libgklayout.dylib 0x198cbbbb nsHTMLBodyElement::GetLink(nsAString_internal&) + 31 (nsHTMLBodyElement.cpp:366)
2 libxpcom_core.dylib 0x0135d7c8 NS_InvokeByIndex + 98 (xptcinvoke_unixish_x86.cpp:179)
Flags: blocking1.9?
|
||
Comment 1•18 years ago
|
||
>- aColor.Truncate(); \
>- nsAutoString color; \
>- nscolor attrColor; \
>- if (!GetAttr(kNameSpaceID_None, nsGkAtoms::attr_, color)) { \
>- \
>- nsPresContext *presContext = GetPresContext(); \
>- if (presContext) { \
>- attrColor = presContext->Default##default_(); \
>- NS_RGBToHex(attrColor, aColor); \
>- } \
>- } else if (NS_ColorNameToRGB(color, &attrColor)) { \
>+ aColor.Truncate();
>+ nsAutoString color;
>+ nscolor attrColor;
>+ if (!GetAttr(kNameSpaceID_None, aAtom, color)) {
>+ NS_RGBToHex(defaultAttrColor, aColor);
>+ } else if (NS_ColorNameToRGB(color, &attrColor)) {
The presContext logic when !GetAttr seems to have been removed in the patch -- was this intentional?
No
|
||
Comment 3•18 years ago
|
||
I'm not too happy with the long if statement replacing what was much shorter in the macro, but this code passes the testcase and still avoids having that much code in the macro.
How about changing those Default* APIs in the prescontext to be just one DefaultColor method which takes an atom as a parameter?
Forget about that idea.
You can shrink this by factoring out the NS_RGBToHex call...
|
|
||
Comment 6•18 years ago
|
||
Attachment #258493 -
Attachment is obsolete: true
Don't bother with "ret", just return the value immediately.
|
|
||
Comment 8•18 years ago
|
||
Attachment #258497 -
Attachment is obsolete: true
Comment on attachment 258498 [details] [diff] [review]
not bothering with ret
+ NS_ASSERTION(PR_FALSE, "Unhandled nsGkAtoms::attribute");
Make this NS_ERROR.
Attachment #258498 -
Flags: superreview+
Attachment #258498 -
Flags: review+
|
|
||
Comment 10•18 years ago
|
||
Attachment #258498 -
Attachment is obsolete: true
|
|
||
Comment 11•18 years ago
|
||
Attachment #258597 -
Attachment is obsolete: true
|
||
Comment 12•18 years ago
|
||
mozilla/content/html/content/src/nsHTMLBodyElement.cpp 1.163
Status: NEW → RESOLVED
Closed: 18 years ago
Flags: blocking1.9?
OS: Mac OS X → All
Hardware: PC → All
Resolution: --- → FIXED
|
|
||
Updated•18 years ago
|
Flags: in-testsuite?
|
|
||
Comment 13•18 years ago
|
||
Attachment #259106 -
Flags: review?(roc)
Attachment #259106 -
Flags: review?(roc) → review+
|
|
||
Comment 14•18 years ago
|
||
Landed the test.
mozilla/content/html/content/test/Makefile.in 1.9
mozilla/content/html/content/test/test_bug373589.html 1.1
Flags: in-testsuite? → in-testsuite+
|
||
Updated•13 years ago
|
Crash Signature: [@ nsPresContext::DefaultLinkColor]
|
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•