Refactor ApplyMinMaxConstraints in nsBlockFrame

RESOLVED FIXED in mozilla17

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jwir3, Assigned: fantasai)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
ApplyMinMaxConstraints in nsBlockFrame applies computed min/max height and width. We want to do these together in only one place. Instead, we should refactor this to be two functions: ApplyMinMaxHeight and ApplyMinMaxWidth.
(Assignee)

Comment 1

5 years ago
Created attachment 643127 [details] [diff] [review]
patch
(Assignee)

Updated

5 years ago
Attachment #643127 - Flags: review?(roc)
Comment on attachment 643127 [details] [diff] [review]
patch

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

::: layout/generic/nsHTMLReflowState.h
@@ +418,4 @@
>     */
> +  nscoord ApplyMinMaxWidth(nscoord aWidth) const {
> +    if (NS_UNCONSTRAINEDSIZE != mComputedMaxWidth)
> +        aWidth = NS_MIN(aWidth, mComputedMaxWidth);

{}

@@ +426,5 @@
> +   * size computed so far.
> +   */
> +  nscoord ApplyMinMaxHeight(nscoord aHeight) const {
> +    if (NS_UNCONSTRAINEDSIZE != mComputedMaxHeight)
> +        aHeight = NS_MIN(aHeight, mComputedMaxHeight);

{}
Attachment #643127 - Flags: review?(roc) → review+
(Assignee)

Comment 3

5 years ago
I left out the extra braces to keep it extra compact since it's inlined in the header. Are you sure you want them in?
Yes. Keeping it compact doesn't matter.
(Assignee)

Comment 5

5 years ago
Created attachment 643362 [details] [diff] [review]
patch with review comments addressed
Attachment #643127 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Comment 6

5 years ago
fantasai, if you add a commit message to your patch, I'll check it in for you.
(Reporter)

Comment 7

5 years ago
Nevermind, I've added it and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e038e82f416
Keywords: checkin-needed
Target Milestone: --- → mozilla17

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/1e038e82f416
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.