391034 - [FIX]"ASSERTION: Unexpected left/right/top/bottom unit" with getComputedStyle on "position: relative; left: 3ch;" element

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Unexpected left/right/top/bottom unit: 'Error', file /Users/jruderman/trunk/mozilla/layout/style/nsComputedDOMStyle.cpp, line 2726

This reminds me of bug 379405.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible fix

This does fix the bug... but I wonder about the behavior here in general.  In particular, the behavior of returning 0 if the offsets are auto, or are percentages and there's no containing block, as well as the "look at the opposite side" thing.  I think it might make more sense to just use SetValueToCoord here and be done with it...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: The right diff

Comment 3

[Boris Zbarsky [:bzbarsky]]

That patch depends on bug 391221.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 275607 [details] [diff] [review]
The right diff

>+  // XXXbz I wish there were some centralized place that had the
>+  // condition of this |if| statement.
>   if (coord.GetUnit() != eStyleUnit_Coord &&
>-      coord.GetUnit() != eStyleUnit_Percent) {
>+      coord.GetUnit() != eStyleUnit_Percent &&
>+      coord.GetUnit() != eStyleUnit_Chars) {

Can you just check coord.GetUnit() == eStyleUnit_Auto?

(Was this one of the places where there used to be some eStyleUnit_Null?)

r+sr+a=dbaron

(Didn't really look at the test, but I trust you there.  I hope you generally test that the test fails without the patch?)

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Can you just check coord.GetUnit() == eStyleUnit_Auto?

Um.... yes.  ;)

And yes, I do check that the tests fail without the patch.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Checked in with that change.  Fixed.

Comment 7

[Jesse Ruderman]

Crashtest checked in.