Closed Bug 315483 Opened 15 years ago Closed 15 years ago

crash on getComputedStyle getting certain properties on an element that's display:none [@ nsHTMLReflowState::GetContainingBlockFor]

Categories

(Core :: DOM: CSS Object Model, defect)

x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: brewt-bugzilla.mozilla.org, Assigned: jlurz24)

References

()

Details

(Keywords: crash, fixed1.8.1, testcase)

Crash Data

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051107 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8) Gecko/20051107 Firefox/1.5

Here's the situation:
- a div that has two styles: display: none, and padding: 10%
- javascript that uses getComputedStyle to return 'padding-left' will crash the browser

If you attempt to get the property 'padding', it doesn't crash (shown by the "what's the padding?" link).  Also, if the div is display:block, then the crash doesn't occur.

Reproducible: Always

Steps to Reproduce:
1. Visit testcase url
2. Click on the "What's the left padding?" link

Actual Results:  
The browser crashes.

Expected Results:  
A javascript alert with the padding value should be shown.
I forgot to add that if the padding is changed to a different unit (eg. px), it doesn't crash.
I crashed with Seamonkey 1.5a rv:1.9a1 build 2005110609 under XP Pro SP2:
Talkback id: TB11576709X

I also crashed with Firefox 1.5 rv:1.8 build 20051107:
Talkback id: TB11576836Y

Adding talkbackid keyword

I have not checked for duplicate yet.
Keywords: crash, talkbackid
Incident ID: 11576836
Stack Signature	nsHTMLReflowState::GetContainingBlockFor 717b16ce
Product ID	Firefox15
Build ID	2005110708
Trigger Time	2005-11-07 18:29:28.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (00207970)
URL visited	
User Comments	Bug 315483
Since Last Crash	7150 sec
Total Uptime	7150 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 457
Stack Trace 	
nsHTMLReflowState::GetContainingBlockFor  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 457]
CalcSidesFor  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/style/nsStyleStruct.cpp, line 132]
nsStylePadding::CalcPaddingFor  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/style/nsStyleStruct.cpp, line 419]
nsComputedDOMStyle::GetPaddingWidthFor  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/style/nsComputedDOMStyle.cpp, line 3326]
nsComputedDOMStyle::GetPropertyValue  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/style/nsComputedDOMStyle.cpp, line 280]
XPTC_InvokeByIndex  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
XPCWrappedNative::CallMethod  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2139]
XPC_WN_CallMethod  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1444]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1177]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3523]
js_Execute  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1424]
JS_EvaluateUCScriptForPrincipals  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 4103]
nsJSContext::EvaluateString  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1061]
nsJSThunk::EvaluateScript  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 297]
nsJSChannel::InternalOpen  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 550]
nsJSChannel::AsyncOpen  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 522]
nsURILoader::OpenURI  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/uriloader/base/nsURILoader.cpp, line 915]
nsDocShell::DoChannelLoad  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 6953]
nsDocShell::DoURILoad  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 6811]
nsDocShell::InternalLoad  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 6578]
nsWebShell::OnLinkClickSync  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsWebShell.cpp, line 549]
OnLinkClickEvent::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsWebShell.cpp, line 345]
PL_HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/xpcom/threads/plevent.c, line 689]
0x778b0c24
nsFormControlList::RemoveElementFromTable  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLFormElement.cpp, line 1994]
0xffffff74
Keywords: talkbackid
Whiteboard: DUPEME
The stack I get from the testcase is very similar to the one in bug 273458 attachment 168555 [details] (which is supposedly fixed). Might be a dupe of bug 194952.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
Attached patch patch_v1Splinter Review
Simple one-liner to fix the crash. This was the only place where GetContainingBlockFor was called without checking for a null frame first.

Also, I agree with the XXX comment. That function shouldn't be inline.
Attachment #228539 - Flags: review?(bzbarsky)
Summary: crash on getComputedStyle getting certain properties on an element that's display:none → crash on getComputedStyle getting certain properties on an element that's display:none [@ nsHTMLReflowState::GetContainingBlockFor]
Comment on attachment 228539 [details] [diff] [review]
patch_v1

I guess this is good enough, yeah.  r+sr=bzbarsky.

Do you need this checked in?
Attachment #228539 - Flags: superreview+
Attachment #228539 - Flags: review?(bzbarsky)
Attachment #228539 - Flags: review+
If you could check this in that'd be great. Thanks.
Assignee: general → jlurz24
Checked in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: blocking1.8.1?
Can we get this on the branch ASAP?
Flags: blocking1.8.1? → blocking1.8.1+
Attachment #228539 - Flags: approval1.8.1?
Target Milestone: --- → mozilla1.8.1beta2
Attachment #228539 - Flags: approval1.8.1? → approval1.8.1+
Checking in nsStyleStruct.cpp;
/cvsroot/mozilla/layout/style/nsStyleStruct.cpp,v  <--  nsStyleStruct.cpp
new revision: 3.129.6.3; previous revision: 3.129.6.2
done

Checked into 1.8.1 branch.
Keywords: fixed1.8.1
Whiteboard: DUPEME
Crash Signature: [@ nsHTMLReflowState::GetContainingBlockFor]
You need to log in before you can comment on or make changes to this bug.