Closed Bug 281630 Opened 15 years ago Closed 13 years ago

Support ch width units in XUL

Categories

(Core :: XUL, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(2 files, 2 obsolete files)

They currently only work in HTML, so I shamlessly copied the code in
nsHTMLReflowState::ComputeHorizontalValue for my patch.
Attached file Test case
Attached patch Proposed patch (obsolete) — Splinter Review
Assignee: nobody → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #173842 - Flags: superreview?(bzbarsky)
Attachment #173842 - Flags: review?(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....
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 on attachment 173842 [details] [diff] [review]
Proposed patch

r-, per comments
Attachment #173842 - Flags: superreview?(bzbarsky)
Attachment #173842 - Flags: superreview-
Attachment #173842 - Flags: review?(bzbarsky)
Attachment #173842 - Flags: review-
Attached patch Updated for bitrot (obsolete) — Splinter Review
OK, so maybe I'm being too eager ;-)
Attachment #173842 - Attachment is obsolete: true
Attachment #263868 - Flags: superreview?(bzbarsky)
Attachment #263868 - Flags: review?(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?
Attachment #263868 - Flags: superreview?(bzbarsky)
Attachment #263868 - Flags: superreview-
Attachment #263868 - Flags: review?(bzbarsky)
Attachment #263868 - Flags: review-
Attachment #263868 - Attachment is obsolete: true
Attachment #263993 - Flags: superreview?(bzbarsky)
Attachment #263993 - Flags: review?(bzbarsky)
Comment on attachment 263993 [details] [diff] [review]
Addressed review comments

Looks good.
Attachment #263993 - Flags: superreview?(bzbarsky)
Attachment #263993 - Flags: superreview+
Attachment #263993 - Flags: review?(bzbarsky)
Attachment #263993 - Flags: review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.