Closed Bug 1177614 Opened 4 years ago Closed 4 years ago

provide a utility method to get containerWidth from an nsHTMLReflowState

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

To support logical-coordinate layout with right-to-left directionality (either horizontal with dir=rtl, or vertical-rl writing mode), we often need a containerWidth value to pass to logical/physical conversion APIs.

This typically comes from the nsHTMLReflowState's ComputedWidth() plus PhysicalBorderPadding().LeftRight(), except that if ComputedWidth() is unconstrained, we use zero instead (to avoid arithmetic involving unconstrained sizes) and fix up the positioning later.

We should have a utility function to encapsulate the common code pattern of getting this containerWidth value from a reflow state. (See bug 1177606 comment 2.)
Blocks: 1131451
It seems like the most natural way to do this is as a method on nsHTMLReflowState. I'm not entirely happy with the name, but I've toyed with various ideas and this is where I settled for now; alternative suggestions welcome.
Attachment #8631548 - Flags: review?(dholbert)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8631548 [details] [diff] [review]
Provide a utility method on nsHTMLReflowState to return the computed size including border-padding, for use as a container for logical coordinate conversions, or zero if unconstrained

Review of attachment 8631548 [details] [diff] [review]:
-----------------------------------------------------------------

r=me; just one nit which may or may not require any action:

::: layout/generic/nsHTMLReflowState.h
@@ +451,5 @@
> +  // unconstrained dimensions replaced by zero.
> +  nsSize ComputedSizeAsContainerIfConstrained() const {
> +    return nsSize(ComputedWidth() == NS_UNCONSTRAINEDSIZE
> +                  ? 0 : ComputedWidth() +
> +                        ComputedPhysicalBorderPadding().LeftRight(),

So, this function makes 2 calls to each of these functions: ComputedWidth(), ComputedHeight(), & ComputedPhysicalBorderPadding().

This looks like repeated work, but right now these are all just accessors for member-vars, so it's not actually problematic for the moment.

BUT: are we planning on making the member-data logical & converting the accessors to be more complex (checking the writing-mode) at some point? If so, we probably should plan for that now & cache the results of these function calls in local variables to avoid redundant calls here. But if not, this seems fine.
Attachment #8631548 - Flags: review?(dholbert) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/b0f47defe8106f673aea5fdd4f91e64aaa5e6ff5
changeset:  b0f47defe8106f673aea5fdd4f91e64aaa5e6ff5
user:       Jonathan Kew <jkew@mozilla.com>
date:       Thu Jul 16 10:07:46 2015 +0100
description:
Bug 1177614 - Provide a utility method on nsHTMLReflowState to return the computed size including border-padding, for use as a container for logical coordinate conversions, or zero if unconstrained. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/b0f47defe810
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.