w3c / csswg-drafts

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

[css-sizing] percentage [max-]width|height and intrinsic sizes #765

Closed MatsPalmgren closed 7 years ago

MatsPalmgren commented 7 years ago

Background: https://lists.w3.org/Archives/Public/www-style/2015Dec/0117.html I'm quoting @dbaron's post here verbatim for convenience:

From: L. David Baron dbaron@dbaron.org Date: Tue, 8 Dec 2015 21:51:01 -0500 To: Karl Dubost kdubost@mozilla.com Cc: www-style@w3.org Message-ID: 20151209025101.GA26911@pescadero.dbaron.org

On Tuesday 2015-10-27 09:44 +0900, Karl Dubost wrote:

Gecko (Firefox) has a Webcompat issue related to the use of max-width inside table. I put an example on Codepen [1].

For Webcompat reasons with Blink and WebKit, we will probably need to modify Gecko code (see Bugzilla [2]). In the seeAlso section of this bug, you will find some of the reported Web Compat issues.

What I would like to know is if the CSS specification needs to be changed to reflect the reality. [...] Currently Blink and Safari are reducing the image so it fits the viewport. You can see this in recent Web Compat bugs https://webcompat.com/issues/1838 https://webcompat.com/issues/1837

I summarized the issue in the webcompat space [4].

So the underlying behavior here that needs to be specified is (using css-sizing terminology) that when either 'width' or 'max-width' on a replaced element is a percentage, that element's min-content contribution is zero. Note that given that this occurs for 'max-width', this needs to override the rule that the min-content contribution is determined by the specified size, since a replaced element with "width: 100px; max-width: 50%" has a min-content contribution of 0.

The relevant section of the specification is: http://dev.w3.org/csswg/css-sizing/#replaced-intrinsic

At least, that's the relevant specification assuming a relatively strict definition of replaced element. This rule should not apply to form controls and similar things that aren't explained by CSS. It should only apply to things that do replaced element sizing such as images, plugins, videos, and iframes.

I'm not sure if we have a clear enough definition of replaced element, or whether we perhaps need two definitions.

-David

MatsPalmgren commented 7 years ago

@dbaron changed our layout in Gecko in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=823483 by reverse-engineering the Chrome/Edge behavior (IIUC). However, there are still some case where UAs are not compatible. When using vertical writing-mode for example: https://bugzilla.mozilla.org/show_bug.cgi?id=1308929

It would be great to actually have a specification for this so that everyone knows what to implement, and have this documented for authors.

MatsPalmgren commented 7 years ago

I think the upcoming introduction of Grid layout in some browsers makes this more urgent to address; given the symmetric nature of track sizing in Grid, it's likely this will lead to web-compat problems. Here are a few simple examples where Firefox / Chrome differs currently: https://people-mozilla.org/~mpalmgren/tests/grid/grid-item-image-size-percentages.html

Fwiw, I tend to think that max-size/size should work symmetricly in both axes.

wolfbeast commented 7 years ago

I'd definitely prefer a definitive spec for this as well, since the undefined behavior makes it possible to implement it in any number of ways that are all logical, but aren't always compatible or giving the expected results to web designers.

fantasai commented 7 years ago

So, from what Tab and I understand, the proposal is to have image-like replaced elements contribute zero as their min-content and max-content size contributions if they have a percentage as either their specified or max size.

There is an open question on whether this should apply only to the inline axis or to both axes (and in the former case which element is determining the axis mapping).

We're happy to make the necessary changes, but we need

  1. confirmation that this is in fact what you're proposing :)
  2. clarity on which axes are affected (we prefer being symmetric if possible, especially in light of Grid)
  3. WG resolution
  4. a decent definition of "image-like" that separates it from form controls (see #1044)

Waiting on your response to the first two, and then we'll Agenda+.

dbaron commented 7 years ago

I was proposing that this change affect only min-content sizing and not max-content sizing. I believe changing max-content sizing in this way would break Web-compatibility.

I believe this needs a definition slightly broader than image-like; for us it was essentially img, canvas, plugins (object/embed), video, and iframe. I believe that list was interoperable, although it's perhaps worth retesting.

(The rule also applies to <textarea> and <input type="text">, although only for percentage width and not for percentage max-width. I believe this was also pretty interoperable.)

It's not clear to me what doing the same for both axes means; I don't think there is such a thing as a min-content size in the block axis.

Or is there a question as to whether the height property should also (in English) affect the min-content size contribution?

fantasai commented 7 years ago

Testcase: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4905 Renderings: https://developer.microsoft.com/en-us/microsoft-edge/tools/screenshots/?url=http%3A%2F%2Fsoftware.hixie.ch%2Futilities%2Fjs%2Flive-dom-viewer%2F%3Fsaved%3D4905

This is various kinds of insane. I understand wanting to have percentage width and/or max-width make the min-content size zero. I could also understand wanting to do this for images but not form controls or vice versa. But I don't understand wanting to do it for both width and max-width for images, but only width (and not max-width) for form controls.

Do we have to be randomly inconsistent here, or can we spec something somewhat sane like "all replaced elements are affected by both max-width and width" or suchlike?

Generally-speaking, the min-content size in the block axis is (like the max-content size) the content-based size--what results from height: auto. We define the concept so that things like grid layout and orthogonal flows can be defined with symmetry. So, for example, if I ask a grid to size a row filled with images or form controls as “min-content”, it's defined. Similarly if I wrap a float around a vertical-rl form control, its min-content width (logical height) needs to be defined.

Also, one thing to point out here: for form controls the min-content contribution isn't exactly zero, it's UA-defined based on how narrow the form control can actually get. This limitation is probably worth preserving in either case.

fantasai commented 7 years ago

Proposal atm is:

The key question is, is this Web-compatible, given form controls don't currently respond to max-width?

dbaron commented 7 years ago

I think the phrasing around "maximally-squashed" might just be "add the padding and border and margin outside of the zero width".

astearns commented 7 years ago

We discussed this on the call and asked some implementors to look at the web compatibility of fantasai's proposal above

gregwhitworth commented 7 years ago

Ok, so based on the @fantasai statement above, to determine the "rough" web compatibility of this we need to essentially look for max-width being used on form controls, correct?

gregwhitworth commented 7 years ago

Anything beyond that - because I'll kick off a crawl looking for this and provide the data here - if you want something more let me know by EOD as I'll kick it off tomorrow. cc: @dbaron @fantasai @MatsPalmgren @astearns

gregwhitworth commented 7 years ago

@fantasai @dbaron Ok here is my recipe I plan to run with: https://github.com/MicrosoftEdge/css-usage/blob/master/src/recipes/max-height-replaced-elems.js

This does what I was saying in IRC that this will determine if a % is used on max-height and is on a replaced element with a height of auto. This does not go the further step of ensure that a parent somewhere up the tree has its height constrained to show this issue. This is more computationally intensive and will slow the crawl down, I think it would be good to cast our net wide and then scope it in. I can actually take the resulting URLs of the wide net and then scope the crawl to only the sites that have the potential problem and look for the actual issue. That said, even upon determining this we'll need to determine how many nodes we need to look up in the tree (if we limit it at all) and then also how we deem if the result would overflow causing the compat issue seen with max-width.

We can figure this out after the crawl is complete. Please let me know ASAP if there is any concern with the recipe I linked to above.

dbaron commented 7 years ago

Not sure why you're looking at height rather than width. I think the idea that there are intrinsic sizes in the block dimension is a spec fiction that I'm working on an alternative proposal for. But any issues in the wild would almost certainly be with width, not height.

Also, I think the proposed change to the widely-implemented behavior affects only textarea and input type="text", not img, since the proposal is to change them to match the img, video, etc., behavior.

gregwhitworth commented 7 years ago

I must have mis-read what @fantasai meant by

Same constraint is also applied in the height dimension.

I assumed she was wanting to know about web compat concerns to make the block direction match the inline. My bad, this is easy enough to modify.

Here is the updated file: https://github.com/MicrosoftEdge/css-usage/blob/master/src/recipes/max-width-replaced-elems.js

dbaron commented 7 years ago

So, the results of a slightly-more-careful review this time:

So it still needs to test somewhere that max-width is a percentage. And it should probably also allow width to be explicitly auto rather than requiring it to be unspecified (which I assume is what the undefined test is checking).

gregwhitworth commented 7 years ago

Wow, I'm surprised I missed the not checking % - gosh. Added that in. https://github.com/MicrosoftEdge/css-usage/blob/master/src/recipes/max-width-replaced-elems.js

gregwhitworth commented 7 years ago

Ok, following up on my action - here is what the crawl found:

Crawl info:

To follow this up for manual inspection, I checked of these URLs if they had a parent with a potential shrink-to-fit parent (abspos, float, table cell, flex).

12,265 of them remain with potential. I did NOT check to see if the parent was setting its own width or if the found element's siblings would have caused the problem to not be noticeable by the end user.

We will follow up on how best we want to determine if this problem will be visible by the end user. @fantasai had some ideas.

gregwhitworth commented 7 years ago

Oh, it's worth noting since @fantasai and I had discussed the possibility of manually looking at these pages to extrapolate on I went ahead and grabbed the ID/classes of the parent, as well as the outerHTML of the element that was using max-width with a %. This should aid in finding the parent quickly and then flooring the s2f to see if the input would overflow with adverse affects on the overall layout to the end user.

fantasai commented 7 years ago

@gregwhitworth Can you attach a textfile with the list of potentially-affected URLs? We can manually check a selection of them.

gregwhitworth commented 7 years ago

max-width.xlsx

Ok, @fantasai I have attached it. Let me explain how the columns relate.

Take the following:

TEXTAREA parent MAIN classes main,col-md-8

This should be read that we found the <textarea> matched what we were looking for. To help out in the manual investigation we then are providing the classes for the parent tag which is a <main> element. The classes are main and col-md-8. So the count row then provides the count of textarea that were found that matched on the page. Then the id row provides the id of the parent if it existed (or it's just empty).

I did reach out to our central data science team regarding how to extrapolate on this and they said, without deeper insights that a good starting place is to select 2% of random URLs and review those. If I and this team, have the time I'll try to get a deeper more specific answer to this question so that we can make this calls with an even higher confidence.

Disclaimer - this data is gathered from the web with as limited bias as possible so there may be some offensive sites included in the results.

fantasai commented 7 years ago

OK, just to clarify, this is a list of pages that have 'width: auto; max-width: ' on either an INPUT or TEXTAREA, correct?

Probably the easiest way to test would be to use a user stylesheet with

input,textarea { width: 0 !important; }

and see if any of the pages appear broken on a random sample of 260 pages from the list.

fantasai commented 7 years ago

(Aside from having unusably narrow input fields, of course.)

gregwhitworth commented 7 years ago

Yes, however this is the reduced list as I stated above. I filtered the initial one further by checking if the input or textarea has an ancestor that is potentially shrink to fit (table-cell, float, abspos, flex, etc). I am not checking if this ancestor has a width set or any children that may cause the resulting negative behavior to exist. But I didn't want us looking through sites that didn't even potentially have the capability of showing the behavior. If you'd like I can do an additional run ensuring that the ancestor does not have a specified width set.

gregwhitworth commented 7 years ago

For example, I just checked out http://57tours.de/ and found the parent that contains the form at the bottom, it's floated to the left but has a defined width of 49.2%. So if we were to floor the input the inputs & textareas you result in what you said in your last comment where the only impact is unusably narrow fields.

FremyCompany commented 7 years ago

Just adding this here since I randomly stumbled onto it but it looks like we actually do zero the min-content-width of some form controls in Edge in some cases, for example as in http://wptest.center/#/udysb8 (this is a reduction of https://beam.pro/ -- the volume slider of the video that loads automatically)

Our behavior enables the author to defines its own min-content-width (but that also means that if they forget, the control collapses, though it's kinda their fault since they did set width:100%). On the Beam's website, this results in an interop difference between Edge and the other browsers, though we seem to do what the author actually intended here (nothing is broken in any browser, someone just wondered why the sizing was slightly different).

dbaron commented 7 years ago

The relevant Gecko code (pemalinks to code as-of-today) is the definition of which form controls to apply the behavior to and the code where we ignore percentage sizes in some cases based on that.

css-meeting-bot commented 7 years ago

The CSS Working Group just discussed percentage [max-]width|height and intrinsic sizes.

RESOLVED: take the change "However, in the case of a replaced box with a percentage-based width/max-width/height/max-height, the percentage is resolved to zero when calculating the min-content contribution in the corresponding axis."

The full IRC log of that discussion <astearns> topic: percentage [max-]width|height and intrinsic sizes
<dbaron> github: https://github.com/w3c/csswg-drafts/issues/765
<tantek> TabAtkins: intrinsic size of some elements with a % width or height
<tantek> fantasai: special behavior that images and form controls have
<tantek> fantasai: if you specify their width as a %, then their min-content size is assigned as if 0 sized
<tantek> fantasai: so this behavior has not been spec'd anywhere and should be spec'd
<tantek> fantasai: the one open consideration, form controls did not do this for max-width, but images did. so images responded to max-width, but form controls didn't
<tantek> fantasai: TabAtkins and I decided to do min and max for both images and form controls
<tantek> fantasai: e.g. you hav especiied mx widht % on a form control. the form control is inside shrink-wrapped container, and the form conrol is affecting the size of the shrink-wrapped container
<tantek> fantasai: seems like a really obscure case and I haven't found any. there may be some out there
<tantek> fantasai: after talking with dbaron it seems that would be ok. want wg resolution to make changes to the spec
<fantasai> s/e.g./the only affected case would be/
<tantek> s/hav especiied mx widht/have specified max width
<tantek> astearns: I want to see the changes in the spec so we can see them in the spec
<tantek> astearns: and discuss them there
<tantek> dbaron: to be clear, the thing that browsers do today isn't in the spec
<tantek> dbaron: tab & fantasai are proposing something simpler but still fairly consistent
<tantek> astearns: are you interested in changing your behavior?
<tantek> dbaron: willing to. have been running with a patch and haven't found anything broken yet
<tantek> dbaron: other part hard here is defining replaced elements
<tantek> TabAtkins: apparently a commit went in today to HTML to define them
<tantek> fremy: on some we are even more inconsistent
<tantek> fantasai: for the sake of sanity we'd like to have ...
<TabAtkins> https://github.com/whatwg/html/pull/2857 PR to define replaced elements
<fantasai> s/.../consistency/
<tantek> fremy: e.g. video element with a slider
<tantek> fremy: all browser but edge, the default size of the contol
<tantek> fremy: meanwhile in edge, default size of 0
<fantasai> Proposed text is "However, in the case of a replaced box with a percentage-based width/max-width/height/max-height, the percentage is resolved to zero when calculating the min-content contribution in the corresponding axis."
<fantasai> in https://drafts.csswg.org/css-sizing/#intrinsic-contribution
<tantek> fantasai: we're not ...
<fantasai> s/fantasai: we're not ...//
<tantek> fremy: there was a slider for the video element which was sized with max-width %
<tantek> fremy: in edge since do we apply ... to this size, the minimum size was ... px, and the author provided size was 41px
<tantek> fremy: in other browsers, they just use the normal default size of the control which was bigger than 41px
<tantek> fremy: I do believe the edge behavior is what the author wanted
<tantek> astearns: if we take this change then ...
<tantek> fremy: all browsers will behave as edge is behaving
<fremy> beam.pro
<tantek> astearns: any objections to taking this change?
<tantek> dbaron: FWIW I just checked the WHATWG HTML dfn and it's not the one we want
<tantek> dbaron: it's the stricter one
<fremy> well, the website has been redesigned
<tantek> dbaron: it doesn't count form elements
<tantek> dbaron: only images, iframes, input type=image
<dbaron> and a bunch of other things
<tantek> astearns: I'm hearing no objection to making the change
<fantasai> I think we need to add "(including form controls)" here
<tantek> astearns: RESOLVED: take the change "However, in the case of a replaced box with a percentage-based width/max-width/height/max-height, the percentage is resolved to zero when calculating the min-content contribution in the corresponding axis."
fantasai commented 7 years ago

@dbaron / @MatsPalmgren Would you mind reviewing the edits to ensure they are correct? The section is https://drafts.csswg.org/css-sizing-3/#intrinsic-contribution and the proposed edits are:

However, in the case of a replaced box with a percentage-based width/max-width/height/max-height, the percentage is resolved to zero when calculating the min-content contribution in the corresponding axis. For example, an input assigned width: calc(50% + 50px) has a min-content contribution of 50px plus any horizontal margin/border/padding.

However, I think it wasn't clearly discussed whether the calc() case should resolve to 50px vs. zero. The alternate set of edits would be

However, in the case of a replaced box with a percentage-based width/max-width/height/max-height, the percentage-based size is treated as zero when calculating the min-content contribution in the corresponding axis. For example, an input assigned width: calc(50% + 50px) has a min-content contribution of zero plus any horizontal margin/border/padding.

tabatkins commented 7 years ago

It would be clearly, fundamentally, obviously wrong if calc(50% + 50px) ever resolved to zero just because percentages resolve against a zero size. I don't think we seriously need to discuss that.

fantasai commented 7 years ago

Testcase: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5297 Results: FF and Edge zero out the percentage only. Chrome zeroes out the entire size but only on images?

@tabatkins Sounds good to me. :) I'll close the issue, then, esp. since the WG resolution explicitly includes that wording and you represent Chrome, which is the browser that (afaict) gets it wrong. :grin:

dbaron commented 7 years ago

I'm certainly OK with trying the first set of edits.

The biggest issue with them is that they need a clearer definition of replaced element. This is the definition of replaced elements that includes not only the ones that honor CSS 2.1's rules on sizing of replaced elements, but also includes most form controls, but not buttons. So I think it's <input> with type other than button/submit/reset, and also <select>, <textarea>, <progress>, and <meter>. I'm not sure about <output>, and I think <fieldset> doesn't count (but it's probably worth testing). The fact that buttons are not included means there isn't an appropriate canned definition floating around.

Another note is that if your testcase determined that Firefox zeroes out only the percentage, then your testcase is wrong, since the last time we talked about this I wrote a patch (still unlanded) to change that.

Also, I filed a Mozilla bug 1388840 on updating the behavior.

fantasai commented 7 years ago

Another note is that if your testcase determined that Firefox zeroes out only the percentage, then your testcase is wrong, since the last time we talked about this I wrote a patch (still unlanded) to change that.

That's what I remembered... was very confused when I didn't get that result. Figured it out, here's the updated testcase: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=5298

dbaron commented 7 years ago

So what about the bit about pointing to an appropriate definition of replaced elements?

fantasai commented 7 years ago

Filed that one as #1889 since this issue is plenty long enough and the issue as initially described has been fixed. Please only reopen if you have an objection to the resolution or the edits implementing it; defining “replaced element” is a whole other realm of problem.

fantasai commented 7 years ago

Fwiw, we checked in edits for #1889 based on @dbaron's comment and would appreciate a review. Thanks~