Closed
Bug 287837
Opened 20 years ago
Closed 19 years ago
###!!! ASSERTION: Unexpected line-height unit
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: MatsPalmgren_bugz, Assigned: Biesinger)
References
Details
(Keywords: assertion)
Attachments
(1 file, 1 obsolete file)
3.50 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
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
Comment 1•20 years ago
|
||
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?
Comment 2•19 years ago
|
||
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).
Comment 3•19 years ago
|
||
So I sort of wonder why we're asserting here... "normal" is a pretty reasonable
value for line-height, no?
Assignee | ||
Comment 4•19 years ago
|
||
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...
Assignee: mats.palmgren → cbiesinger
Status: NEW → ASSIGNED
Attachment #196254 -
Flags: superreview?(bzbarsky)
Attachment #196254 -
Flags: review?(bzbarsky)
Comment 5•19 years ago
|
||
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...
Attachment #196254 -
Flags: superreview?(bzbarsky)
Attachment #196254 -
Flags: superreview-
Attachment #196254 -
Flags: review?(bzbarsky)
Attachment #196254 -
Flags: review-
Assignee | ||
Comment 6•19 years ago
|
||
well, if I do that I get:
###!!! ASSERTION: not initial state: 'val->GetUnit() == eCSSUnit_Null', file
../../../../mozilla/layout/style/nsCSSDataBlock.cpp, line 887
Comment 7•19 years ago
|
||
If you do which?
Comment 8•19 years ago
|
||
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.
Assignee | ||
Comment 9•19 years ago
|
||
ah, ok, that makes a lot of sense. will do.
Assignee | ||
Comment 10•19 years ago
|
||
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.
Attachment #196254 -
Attachment is obsolete: true
Attachment #196549 -
Flags: superreview?(bzbarsky)
Attachment #196549 -
Flags: review?(bzbarsky)
Updated•19 years ago
|
Attachment #196549 -
Flags: superreview?(bzbarsky)
Attachment #196549 -
Flags: superreview+
Attachment #196549 -
Flags: review?(bzbarsky)
Attachment #196549 -
Flags: review+
Assignee | ||
Comment 11•19 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Target Milestone: --- → mozilla1.9alpha
You need to log in
before you can comment on or make changes to this bug.
Description
•