Bug 281972 uncovers a couple of bugs in initial values which were previously hidden under a "#ifdef DEBUG_ComputedDOMStyle". STEPS TO REPRODUCE: 1. in a debug build where bug 281972 is fixed - load about:blank and invoke the DOM Inspector 2. select the HTML element and then click "Computed Style" in the right panel and scroll down. ACTUAL RESULTS: ###!!! ASSERTION: Unexpected line-height unit: 'Error', file nsComputedDOMStyle.cpp, line 1689 ###!!! ASSERTION: Unexpected outline radius unit: 'Error', file nsComputedDOMStyle.cpp, line 1503 ###!!! ASSERTION: Unexpected border radius unit: 'Error', file nsComputedDOMStyle.cpp, line 3374

Mats, I can't seem to reproduce this (and the initial value of line-height has been "normal" ever since ruletree landed, as far as I can tell). Are you still seeing this?

I can reproduce this in a recent trunk build (pulled last week). Makes using DOM Inspector quite painful when you have break-on-assert enabled. When I hit the first assertion in nsComputedDOMStyle::GetLineHeight, text->mLineHeight.mUnit is eStyleUnit_Normal so we end up in the NS_ERROR on line 1799 of nsComputedDOMStyle.cpp (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/style/nsComputedDOMStyle.cpp&rev=1.143&root=/cvsroot&mark=1799#1776).

So I sort of wonder why we're asserting here... "normal" is a pretty reasonable value for line-height, no?

how about this? this uses 0 if no explicit value is specified for outline-radius and border-radius (and handles normal for line-height). I'm not sure, though, if that usage of 0 should be done here in nsRuleNode, or in the cssstruct constructor...

Comment on attachment 196254 [details] [diff] [review] patch >Index: nsComputedDOMStyle.cpp > default: > NS_ERROR("Unexpected line-height unit"); >+ case eStyleUnit_Normal: > val->SetIdent(nsLayoutAtoms::normal); > break; Please put that before default, ok? No need to set to "normal" in the default case here, since it really should be notreached with this change. >Index: nsRuleNode.cpp This should be done in the struct constructor, in both cases; your change, as written, will clobber values that should not be clobbered with 0...

well, if I do that I get: ###!!! ASSERTION: not initial state: 'val->GetUnit() == eCSSUnit_Null', file ../../../../mozilla/layout/style/nsCSSDataBlock.cpp, line 887

If you do which?

Oh, wait. I see. I was talking about the _style_ struct, not the css struct. The css struct should default to null, the style struct should default to the initial value, whatever that is.

ah, ok, that makes a lot of sense. will do.

I'm not quite sure that NS_FOR_CSS_SIDES is a good macro to use here, because it uses PRInt32 as the variable type, while nsStyleSides uses PRUint8, but other code in this file uses it too.

Checking in layout/style/nsComputedDOMStyle.cpp; /cvsroot/mozilla/layout/style/nsComputedDOMStyle.cpp,v <-- nsComputedDOMStyle.cpp new revision: 1.144; previous revision: 1.143 done Checking in layout/style/nsStyleStruct.cpp; /cvsroot/mozilla/layout/style/nsStyleStruct.cpp,v <-- nsStyleStruct.cpp new revision: 3.132; previous revision: 3.131 done