Closed
Bug 281630
Opened 20 years ago
Closed 18 years ago
Support ch width units in XUL
Categories
(Core :: XUL, enhancement)
Core
XUL
Tracking
()
RESOLVED
FIXED
People
(Reporter: neil, Assigned: neil)
References
Details
Attachments
(2 files, 2 obsolete files)
|
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
|
(deleted),
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
They currently only work in HTML, so I shamlessly copied the code in
nsHTMLReflowState::ComputeHorizontalValue for my patch.
|
Assignee | |
Comment 1•20 years ago
|
||
|
|
Assignee | |
Comment 2•20 years ago
|
||
Assignee: nobody → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #173842 -
Flags: superreview?(bzbarsky)
Attachment #173842 -
Flags: review?(bzbarsky)
|
||
Comment 3•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
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•18 years ago
|
||
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 7•18 years ago
|
||
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•18 years ago
|
||
Attachment #263868 -
Attachment is obsolete: true
Attachment #263993 -
Flags: superreview?(bzbarsky)
Attachment #263993 -
Flags: review?(bzbarsky)
|
|
||
Comment 9•18 years ago
|
||
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•18 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
|
||
Updated•18 years ago
|
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.
Description
•