Closed Bug 363950 Opened 13 years ago Closed 13 years ago

crash [@ nsComputedDOMStyle::GetMarginWidthCoordFor][@ nsComputedDOMStyle::GetPaddingWidthCoordFor]

Categories

(Core :: Layout, defect, P1, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: Peter6, Assigned: dbaron)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [patch])

Crash Data

Attachments

(2 files, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061215 Minefield/3.0a1 ID:2006121503 [cairo]

trunk builds are crashing on nsComputedDOMStyle::GetMarginWidthCoordFor

Incident ID: 27352316
Stack Signature	nsComputedDOMStyle::GetMarginWidthCoordFor a2e27e9e
Product ID	FirefoxTrunk
Build ID	2006121404
Trigger Time	2006-12-14 23:19:23.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (001b3b47)
URL visited	iwon.com
User Comments	loading a Java game
Since Last Crash	306 sec
Total Uptime	306 sec
Trigger Reason	Access violation
Source File, Line No.	e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\layout\style\nscomputeddomstyle.cpp, line 3554
Stack Trace 	
nsComputedDOMStyle::GetMarginWidthCoordFor  [mozilla\layout\style\nscomputeddomstyle.cpp, line 3554]
nsComputedDOMStyle::GetMarginWidthFor  [mozilla\layout\style\nscomputeddomstyle.cpp, line 3546]
NS_InvokeByIndex  [mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp, line 102]
XPCWrappedNative::CallMethod  [mozilla\js\src\xpconnect\src\xpcwrappednative.cpp, line 2162]

probable regression from Bug 332922
Attached file testcase
It seems to happen when calling the computed style of margin or padding on an element that isn't visible.
Blocks: 332922
Severity: major → critical
Keywords: crash, testcase
Yeah, the aFrame param to all the computed style methods is null in the display:none case...
Flags: blocking1.9?
OS: Windows 2000 → All
Hardware: PC → All
Attached patch patch (obsolete) — Splinter Review
Fixes the crash; needs more testing for correctness.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [patch]
Target Milestone: --- → mozilla1.9beta
Summary: crash [@ nsComputedDOMStyle::GetMarginWidthCoordFor] → crash [@ nsComputedDOMStyle::GetMarginWidthCoordFor][@ nsComputedDOMStyle::GetPaddingWidthCoordFor]
nsMargin has a side() method that might do what you want here, btw.  Help you nix some of those switches.
Comment on attachment 248764 [details] [diff] [review]
patch

Tested by loading a bunch of data URLs in DOM inspector and looking at the computed style, testing the full matrix of:
 * both display:none and not
 * both margin and padding
 * different values on each side

And I mixed up the unit types while doing this.
Attachment #248764 - Flags: superreview?(bzbarsky)
Attachment #248764 - Flags: review?(bzbarsky)
Oh, right, I was wishing that was available and forgot it actually was -- new patch in a sec.
Attached patch patchSplinter Review
Attachment #248764 - Attachment is obsolete: true
Attachment #248781 - Flags: superreview?(bzbarsky)
Attachment #248781 - Flags: review?(bzbarsky)
Attachment #248764 - Flags: superreview?(bzbarsky)
Attachment #248764 - Flags: review?(bzbarsky)
Comment on attachment 248781 [details] [diff] [review]
patch

Looks great!  r+sr=bzbarsky

We should add tests for this to mochitest or something.
Attachment #248781 - Flags: superreview?(bzbarsky)
Attachment #248781 - Flags: superreview+
Attachment #248781 - Flags: review?(bzbarsky)
Attachment #248781 - Flags: review+
Fix checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Status: RESOLVED → VERIFIED
Flags: blocking1.9?
Crash Signature: [@ nsComputedDOMStyle::GetMarginWidthCoordFor] [@ nsComputedDOMStyle::GetPaddingWidthCoordFor]
Crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c9d1fc191cf
Crash Signature: [@ nsComputedDOMStyle::GetMarginWidthCoordFor] [@ nsComputedDOMStyle::GetPaddingWidthCoordFor] → [@ nsComputedDOMStyle::GetMarginWidthCoordFor] [@ nsComputedDOMStyle::GetPaddingWidthCoordFor]
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.