nsHTMLReflowState::InitResizeFlags unnecessarily checks for min-height:auto and max-height:auto

RESOLVED FIXED in mozilla17

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
In bug 763689, I'm adding support for the new min-width / min-height: auto keyword. Before that bug's patches, we don't support "auto" for these properties.

However, nsHTMLReflowState::InitResizeFlags currently has this check:
> 533   bool dependsOnCBHeight =
[...]
> 537     (mStylePosition->MinHeightDependsOnContainer() &&
> 538      // FIXME: condition this on not-abspos?
> 539      mStylePosition->mMinHeight.GetUnit() != eStyleUnit_Auto) ||
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsHTMLReflowState.cpp#533
from this cset:
http://hg.mozilla.org/mozilla-central/rev/fcc2aa4bd451#l6.11

That "mMinHeight.GetUnit() != eStyleUnit_Auto" check is trivially true right now.

Moreover, after bug 763689 lets us have "auto" in computed-style, we explicitly *won't* want the check for 'auto' here. (since the used value of "min-height:auto" is either 0 or the item's intrinsic height -- it never depends on the container's height).

So, I think we want to just drop this line.
(Assignee)

Comment 1

5 years ago
(In reply to Daniel Holbert [:dholbert] from comment #0)
> That "mMinHeight.GetUnit() != eStyleUnit_Auto" check is trivially true right
> now.

That's true of the mMaxHeight check in the same chunk of code, too, actually.

max-height doesn't accept "auto", as shown here:
  http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSPropList.h#2064
(Assignee)

Updated

5 years ago
Summary: nsHTMLReflowState::InitResizeFlags incorrectly checks for min-height:auto → nsHTMLReflowState::InitResizeFlags unnecessarily checks for min-height:auto and max-height:auto
(Assignee)

Comment 2

5 years ago
Created attachment 645896 [details] [diff] [review]
fix v1
Attachment #645896 - Flags: review?(dbaron)
(Assignee)

Updated

5 years ago
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 3

5 years ago
Created attachment 645904 [details] [diff] [review]
fix v1a

(reposting patch with 8 lines of context, instead of 3, to make it clearer)
Attachment #645896 - Attachment is obsolete: true
Attachment #645896 - Flags: review?(dbaron)
Attachment #645904 - Flags: review?(dbaron)
Comment on attachment 645904 [details] [diff] [review]
fix v1a

r=dbaron
Attachment #645904 - Flags: review?(dbaron) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/83a4dfef9a7d
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/83a4dfef9a7d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.