Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement `min-width` and `max-width` #783

Closed
pcwalton opened this issue Aug 26, 2013 · 8 comments
Closed

Implement `min-width` and `max-width` #783

pcwalton opened this issue Aug 26, 2013 · 8 comments

Comments

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Aug 26, 2013

Needed for Acid2 line 1.

@ILyoan
Copy link
Contributor

@ILyoan ILyoan commented Oct 29, 2013

Is anybody working on this? or I'll try to implement this.

@jdm
Copy link
Member

@jdm jdm commented Oct 29, 2013

I am not aware of any prior work.

bors-servo pushed a commit that referenced this issue Nov 6, 2013
For #783, only for block flow.
I'll do the things for other flows as well.

r? @SimonSapin, @metajack
bors-servo pushed a commit that referenced this issue Nov 7, 2013
For #783, only for block flow.
I'll do the things for other flows as well.

r? @SimonSapin, @metajack
@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Dec 10, 2013

Needed for #1366.

@pradeep90
Copy link
Contributor

@pradeep90 pradeep90 commented Feb 4, 2014

We still need to implement min-width and max-width for floats and other elements.
The relevant algorithm is at http://www.w3.org/TR/CSS2/visudet.html#min-max-widths

For non-replaced elements, the algorithm is the same: calculate width normally; if it is greater than max-width, re-calculate using max-width, etc.
BlockFlow::compute_block_margins implements this algo right now.

So, all we would need to do further for non-replaced elements is make compute_block_margins a general function which will call the specific margin-calculation function within it:

  • compute_horiz for blocks
  • compute_float_margins for floats
    and so on
bors-servo pushed a commit that referenced this issue Mar 11, 2014
+ Move out and separate the width calculation code for 6 cases like: block-replaced, block-non-replaced, float-replaced, etc.
  This implements #1683
+ Use the trait to have a common min-width and max-width calculation function (This completes #783 for blocks)
+ Add reftests for the above.
bors-servo pushed a commit that referenced this issue Mar 11, 2014
+ Move out and separate the width calculation code for 6 cases like: block-replaced, block-non-replaced, float-replaced, etc.
  This implements #1683
+ Use the trait to have a common min-width and max-width calculation function (This completes #783 for blocks)
+ Add reftests for the above.
@pcwalton
Copy link
Contributor Author

@pcwalton pcwalton commented Mar 13, 2014

I didn't notice any problems in Acid2 related to this. Is this done enough to move off the milestone?

@pradeep90
Copy link
Contributor

@pradeep90 pradeep90 commented Mar 13, 2014

min-width doesn't even appear in acid2.html
max-width appears only in this line -

.picture p { position: fixed; margin: 0; padding: 0; border: 0; top: 9em; left: 11em; width: 140%; max-width: 4em; height: 8px; min-height: 1em; max-height: 2mm; /* min-height overrides max-height, see 10.7 \
*/ background: black; border-bottom: 0.5em yellow solid; }

I think replaced elements with an intrinsic ratio still need to be handled in the long-term.
But in the above line, it's not a replaced element, so I think we're done for Acid2 min-width and max-width.

@pradeep90
Copy link
Contributor

@pradeep90 pradeep90 commented Mar 13, 2014

Work still left to do in general (not for Acid2):
Handle replaced elements with an intrinsic ratio and both 'width' and 'height' specified as 'auto'.
This involves the different cases in the table in CSS Section 10.4 - http://www.w3.org/TR/CSS2/visudet.html#min-max-widths

@pcwalton
Copy link
Contributor Author

@pcwalton pcwalton commented Sep 16, 2016

I think we should close this in favor of more specific bugs.

@pcwalton pcwalton closed this Sep 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.