search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.44k stars 657 forks source link

[CSS21] error in 10.3.2 Inline, replaced elements #5583

Open cbiesinger opened 3 years ago

cbiesinger commented 3 years ago

https://www.w3.org/TR/CSS2/visudet.html#inline-replaced-width

If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, then that intrinsic width is the used value of 'width'.

This is incorrect; it needs to take into account the transferred min and max height.

cbiesinger commented 3 years ago

@davidsgrogan @tabatkins @fantasai

fantasai commented 3 years ago

In the CSS2 spec, the min/max-size properties are handled as a post-processing step, in which the limit, if applied, gets substituted in as the value of the affected property.

Then apply the rules under "Calculating widths and margins" above, as if 'width' were computed as this value. §10.4

So the spec is correct.

In the CSS3 specs, we try to incorporate the min/max limits into the sizing algorithms. I'm not sure if this is a better or worse approach, but anyway, CSS2 is afaict correct.

cbiesinger commented 3 years ago

@fantasai OK then you need to update other specs. Consider the definition of min-content: https://drafts.csswg.org/css-sizing-3/#sizing-values -> https://drafts.csswg.org/css-sizing-3/#min-content -> https://www.w3.org/TR/CSS2/visudet.html#float-width

No part of this says that for replaced elements, or elements with an aspect ratio, the transferred min/max size should be taken into account.

cbiesinger commented 3 years ago

(i.e. for "width: min-content")

cbiesinger commented 3 years ago

Going to remove the label for now, so that you see my followup question

davidsgrogan commented 3 years ago

I'm going to proceed with implementation as if the transferred min/max is to be obeyed for aspect-ratio elements (maybe cbiesinger and I can't find the relevant spec text, or it's unintentionally unspecified) such that this image is expected to have a width of 50px:

data:text/html,<img src="https://placekitten.com/300/300" style="max-height:50px">

But, @fantasai , please update here if the transferred min/max is to be ignored on elements with an aspect ratio.

fantasai commented 3 years ago

@davidsgrogan The rendering for that image testcase is defined in https://www.w3.org/TR/CSS2/visudet.html#min-max-widths fwiw. It's not undefined, it's been defined since 2004 or thereabouts.

Spec text on 'aspect-ratio' is drafted in https://drafts.csswg.org/css-sizing-4/#aspect-ratio I'm not sure why you say you can't find anything about transferring?

Additionally, sizing constraints in either axis (the origin axis) are transferred through the preferred aspect ratio to the other axis (the destination axis) as follows:

First, any definite minimum size is converted and transferred from the origin to destination axis. This transferred minimum is capped by any definite preferred or maximum size in the destination axis.

Then, any definite maximum size is converted and transferred from the origin to destination. This transferred maximum is floored by any definite preferred or minimum size in the destination axis as well as by the transferred minimum, if any.

davidsgrogan commented 3 years ago

Ah, thanks for pointing out what I missed.

I'm not sure why you say you can't find anything about transferring?

I didn't say that. Note the "maybe." In any case, I had looked but hadn't found it when I wrote that comment. (The corpus of css specs are inherently difficult to follow; I wasn't looking in the right place.)

Substantively, what is the expected width of this variant? I think this is analogous to the case that @cbiesinger referred to above.

data:text/html,<img src="https://placekitten.com/300/300" style="max-height:50px; width: max-content;">

davidsgrogan commented 3 years ago

Actually, I think the case I posted in the last comment is controlled by https://www.w3.org/TR/CSS2/visudet.html#min-max-widths, is it not? I.e.

Start with

<img src="https://placekitten.com/300/300" style="max-height:50px; width: max-content;">

  1. https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes dictates its width of to be the width of

    <div style="width: 9999"><img src="https://placekitten.com/300/300" style="max-height:50px; float: left;">

  2. which https://www.w3.org/TR/CSS2/visudet.html#float-replaced-width defines to be the width of

    <div style="width: 9999"><img src="https://placekitten.com/300/300" style="max-height:50px;">

  3. Which https://www.w3.org/TR/CSS2/visudet.html#inline-replaced-width dictates is 300 because of

    If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, then that intrinsic width is the used value of 'width'.

So, as of here the used width is 300.

  1. But then in applying the max-height https://www.w3.org/TR/CSS2/visudet.html#min-max-heights says

    for replaced elements with both 'width' and 'height' computed as 'auto', use the algorithm under Minimum and maximum widths above to find the used width and height.

  2. For Minimum and maximum widths, the 3rd line of the table, h > max-height, applies. Using its rules of resolved width = max(max-height * w/h, min-width) and resolved height = max-height, used width = used height = 50.

Where did I go wrong?