Support ch width units in XUL

RESOLVED FIXED

Status

()

Core
XUL
--
enhancement
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

(Depends on: 1 bug)

Trunk
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

14 years ago
They currently only work in HTML, so I shamlessly copied the code in
nsHTMLReflowState::ComputeHorizontalValue for my patch.
(Assignee)

Comment 1

14 years ago
Created attachment 173841 [details]
Test case
(Assignee)

Comment 2

14 years ago
Created attachment 173842 [details] [diff] [review]
Proposed patch
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-
(Assignee)

Comment 6

11 years ago
Created attachment 263868 [details] [diff] [review]
Updated for bitrot

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-
(Assignee)

Comment 8

11 years ago
Created attachment 263993 [details] [diff] [review]
Addressed review comments
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+
(Assignee)

Comment 10

11 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite?

Updated

10 years ago
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.