281630 - Support ch width units in XUL

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[[e-mail]]

They currently only work in HTML, so I shamlessly copied the code in
nsHTMLReflowState::ComputeHorizontalValue for my patch.

Comment 1

[[e-mail]]

Attachment: Test case

Comment 2

[[e-mail]]

Attachment: Proposed patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

So the only case in which we support these 'ch' units (which aren't in any spec,
btw, so really should be _moz_ch or something) is when they're specifying a
width?  That seems pretty wrong....

Comment 4

[Boris Zbarsky [:bzbarsky]]

I think we should sort out bug 282126 before taking this patch... that bug is
likely to make this change irrelevant one way or another.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173842 [details] [diff] [review]
Proposed patch

r-, per comments

Comment 6

[[e-mail]]

Attachment: Updated for bitrot

OK, so maybe I'm being too eager ;-)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 263868 [details] [diff] [review]
Updated for bitrot

>Index: layout/xul/base/src/nsBox.cpp
>     // see if the width or height was specifically set
>     if (position->mWidth.GetUnit() == eStyleUnit_Coord)  {
>         aSize.width = position->mWidth.GetCoordValue();
>         widthSet = PR_TRUE;
>+    } else if (position->mWidth.GetUnit() == eStyleUnit_Chars) {
>+        aSize.width = nsLayoutUtils::CharsToCoord(position->mWidth,
>+            aState.GetRenderingContext(), aBox->GetStyleContext());
>+        widthSet = PR_TRUE;
>     }

How about exposing GetAbsoluteCoord on nsStyleUtils instead, and using that here?  Then you could do:

  PRBool widthSet = 
    nsLayoutUtils::GetAbsoluteCoord(position->mWidth,
                                    aState.GetRenderingContext(),
                                    aBox, aSize.width);

And same in the other cases.  Less code duplication and all.

>-    // XXX Handle eStyleUnit_Chars, eStyleUnit_Enumerated?

Leave the eStyleUnit_Enumerated part of this, please?

Comment 8

[[e-mail]]

Attachment: Addressed review comments

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 263993 [details] [diff] [review]
Addressed review comments

Looks good.

Comment 10

[[e-mail]]

Fix checked in.