-
Notifications
You must be signed in to change notification settings - Fork 715
[css-overflow-3] 'overflow' 2-value syntax is in wrong order #2988
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
Comments
It's especially important to keep |
The Working Group just discussed
The full IRC log of that discussion<dael> Topic: 'overflow' 2-value syntax is in wrong order<dael> github: https://github.com//issues/2988 <dael> Rossen_: This is value ordering and we try to do x then y, but all logical properties are block and then inline. What is the proposal here? <dael> fantasai: Proposal is to use the logical order, block then inline. Overflow prop do have logicial shorthands. Also pretty important to make sure ordering matches with scroll-snap-align. Having them opposite is fairly confusing <dael> Rossen_: This would apply to...a handful or properties <dael> fantasai: Just overflow shorthand property. I don't know of any others, though they might exist <dael> Rossen_: Do we keep consistency for position properties? <dbaron> how many engines implement 2-value overflow? Gecko does. <dael> fantasai: Once in logical coord are block first inline second. All the 4 values are block first inline second <dael> fantasai: Logicial 2 value coord is mainly background-position and that's a rough WD with logical positioning values <dael> fantasai: Physical are x then y <dael> dbaron: How many engines impl? <dael> fantasai: Just mozilla b/c we jsut added in April <dael> Rossen_: We've had overflow-x and -y since IE 6 <dael> fantasai: We won't change that. <bradk> When did we resolve on doing block first? That seems wrong to me. <rego> Chromium has it in M68 I believe <dael> Rossen_: Sorry, though you were talking about long hand <rego> https://www.chromestatus.com/feature/5090725653905408 <dael> Rossen_: So that means compat risk is fairly small <dael> fantasai: Yeah <fantasai> bradk, awhile back ... when we were working on css-grid <dael> Rossen_: dbaron would you guys be okay with this? <dael> dbaron: Yeah <dael> Rossen_: Prop: Match the block followed by inline ordering of 2 value pairs for the overflow-x and overflow-y shorthand to be consistent <dael> Rossen_: Objections? <fantasai> bradk, I'm not entirely convinced it was a good idea, there were good arguments in both directions... but we should be consistent here <dael> RESOLVED: Match the block followed by inline ordering of 2 value pairs for the overflow-x and overflow-y shorthand to be consistent |
I filed Mozilla bug 1481866 to change this. |
Blink also implemented (https://bugs.chromium.org/p/chromium/issues/detail?id=833105 is fixed), so I filed https://bugs.chromium.org/p/chromium/issues/detail?id=872356 for them. |
Currently, the 2-axis starts-with-horizontal rule, and the up-to-four-compass-directions starts-with-top rule is understandable and mostly internally consistent. Having some values that follow those rules and others that don’t is going to be horribly confusing and hard to remember. I should have said so in the call. |
Right, but it'll be confusing no matter what, as we have several properties already doing 2-axis in the logical order. The physical order is more or less just a legacy mistake at this point. |
I was about to patch the spec, but now I'm confused: is the shorthand supposed to be overflow-block then overflow-inline (logical longhands), or overflow-y then overflow-x (physical longhands)? The resolution sounds like the former (especially since @fantasai invokes consistency with scroll-snap-align as a rationale), but as far as I can tell the code change by Mozilla and the request in Chrome's bug tracker to match the resolution are the later. It seems to me that they're wrong, but I wasn't there during the call, so I may be missing something. Which is it? |
All shorthands that don't contain |
that seems to contradict what @tabatkins and @fantasai were saying thought.
|
Yes, our entire argument, and the resolution, was to have the order be "block, then inline", not "x, then y". (The resolution is... confusingly written, since it refers to "the overflow-x and overflow-y shorthand" rather than... the "overflow" property, which predates either.
Incorrect (see any of the Grid properties). And doesn't make a ton of sense, since a property that applies to both axises wouldn't specialize its name with "block" or "inline" - that's for sub-properties that are doing a single axis. |
Ok, so I'll fix the spec per the resolution (i.e. in terms of logical directions), and have pinged Chrome and Mozilla in their relevant bugs about the misunderstanding. |
I think that's way riskier than what we interpreted. For example, code like this will stop working: document.body.style.overflow = "hidden";
document.body.style.overflowX // Will start returning "", but returned "hidden" up until now. I'm not sure that's totally acceptable... |
Also, we haven't implemented |
Discussed this a bit on the call:
|
Also, I'm not sure about the shorthand-expands-to-different longhands thing @fantasai mentions. I think that's not a trivial change at all. For example we need to define how the shorthand serialization argument works in that case. If I did: overflow-x: hidden;
overflow-y: hidden;
overflow-block: visible;
overflow-inline: hidden;
That all needs more thought... Not saying that it's impossible to spec or implement, but... |
Sure, grid properties refer to columns or rows which are logical, but there aren't physical properties. In cases with both physical and logical longhands, the shorthand seems to refer to the physical ones. Changing |
Because all of those are legacy shorthands dating back before we started adding logical longhands. In the time since we started doing that, we've defaulted to logical, and in fact have generally done logical-only.
In all current impls
Note that this isn't a backwards-compat question, as this is assuming use of the new logical longhands, nor it is even overflow-specific; this is just a general question about how shorthands with both physical and logical longhands should serialize. (I'm not sure if this question is answered in css-logical offhand, as I'm not an editor - @fantasai or @frivoal?) |
Yes, that's what I don't think it's backwards compatible.
Shorthands only correspond to physical or logical longhands, not both. For example, That's how it has been implemented in Firefox and Blink. (Well, right now Blink never serializes logical shorthands, I'm writing a patch right now). |
Yes, because it needed to be consistent with the rest of the margin/padding/border set.
Shorthands correspond to both. We're still working out the exact mechanism, but they do correspond to both.
Once we have a syntax for representing a logical |
I'm reverting the swap Firefox made in https://bugzilla.mozilla.org/show_bug.cgi?id=1492567. There's no point in shipping |
@emilio I would understand if you reverted to only accepting one value, but reverting to the opposite of the spec says means that we will end up with Web content depending on what you are shipping. I don't think this is a good plan. Either ship something compatible with the spec or object to the spec being incompatible with what you insist on shipping. |
I replied on the bug but:
I'm reverting to what we (and other engines) are shipping, to avoid shipping something that is both incompatible with the spec and with our previous behavior. We've been shipping the shorthand in the x/y form since Firefox 61. If we want to unship that (which I could understand) I can do that, but I definitely would like that to go through the regular path and not getting the change in a beta without any nightly cycle. Also, we should ask the Blink folks to unship their shorthand too in that case. My objection with what the CSSWG seemed to resolve (which both me and David misinterpreted), apart from the ones I've expressed in comments above, is mostly that the WG decided we wanted to change a shorthand to work like no other shorthand currently does (referring to some properties sometimes, and to others other times) after one implementation had already shipped the previously-resolved thing, which is the opposite behavior, and another had already implemented and cleared for shipping the same way. |
Suggestion: 2 shorthands It would seem to be more consistent with rest of css logical. |
which obviously already exist in overflow 3. |
@emilio If you have an objection to what's in the spec, you should raise that issue, not leave an offhand comment in a bug that you've implemented the opposite of the spec without any indication of what you expect the WG or other implementers to do about it. If you go back and read my comment, I said there were two conditions either of which are reasonable positions to take: a) You implement something compatible with the spec. b) You implement something else and ask the spec to change. Your comments read off as c) You implement something incompatible with the spec, and don't ask for the spec to change. I don't think c) is reasonable behavior and consider it bad form. |
To be clear, I have not implemented anything related to this spec change. I did implement the original resolution, which we and Blink were shipping, and I have just reverted a patch which also diverged from this resolution, in order to avoid shipping two different things that diverged from the spec. I don't really think that should be controversial, tbh. |
@tabatkins mentioned this bug today. We should try to change this in Blink before the usage grows large and the compat issue difficult to deal with. Note that we have already switched At the moment I am trying to understand the current usage and the scope of potential breakage. At the moment I am looking at httparchive data (bug) since Blink usage stats are most likely not helpful since this overloads existing single-value |
@gregwhitworth Can https://developer.microsoft.com/en-us/microsoft-edge/platform/usage/css/overflow/ show how often a pair of values are used? |
The CSS Working Group just discussed The full IRC log of that discussion<dael> Topic: 'overflow' 2-value syntax is in wrong order<dael> github: https://github.com//issues/2988 <dael> astearns: IRC chat above about it <dael> fantasai: Looks like waiting on majidvp making change in Blink which requires him doing some research <dael> florian: What are we blocked on majidvp for? <astearns> majidvp> fantasai: no new update. I have not had a chance to work on this since last update. My next step was to look at the mobile data from httparchive. Pending that does not raise a red flag we can try to fix this in Blink. <dael> TabAtkins: He's an interested impl who understand problem and wants to fix. <dael> florian: He's attempting to do what we spec and if he succeeds that overrules the concerns? <dael> TabAtkins: I believe so. I think only concern is compat so us being happy avoids that <dael> emilio: Compat concern, but also shorthands that expand to multiple longhands. Don't know if Blink plans to impl, but need to do more off spec work. <dael> TabAtkins: Yes, that's discussion from TPAC. That's a larger issue that needs to be resolved in sensible way. <dael> TabAtkins: We already have a number of proerties with this problem, so we need to solve for all <dael> emilio: Cannot fix this without figuring out whole thing <dael> TabAtkins: We've already got margin-start and margin-left <dael> emilio: margin shorthand is only physical <dael> TabAtkins: Yes, but need to worry about shorthand intereaction with both. Same thing you've got here. <dael> emilio: It's another level of interaction <dael> TabAtkins: I don't understand how different <dael> florian: Same as having the extra keyword on shorthands to say. We don't have that <dael> TabAtkins: If you set margin and margin-inline-start and ask for margin, we don't know what it should return <dael> emilio: We do <dael> florian: Margin is shorthand of physical only. <dael> fantasai: It's impl that way. If you replace margin with physical shorthands you get corrent. gCS it's mapped across both <dael> emilio: gCS is different b/c knows writing mode. Issue is specified style. This would be a case where there is no answer <dael> TabAtkins: If overflow 2 value is logical and margin is phsyical it's congruint <dael> emilio: When you spec overflow it maps to 4 properties. Overflow shorthand can take different prop <dael> florian: Shorthand to longhand is parse time, but need to parse on computed value <dael> TabAtkins: Way we're talking is it's parse time. Overfloew 2 value expands to logicial long hands. <dael> emilio: But only when in 2 value <dael> TabAtkins: When in 1 doesn't matter <dael> emilio: Does matter because then you need to return something for physical for compat <dbaron> I guess you can't hear me? <dael> TabAtkins: Same problem as margin. Set margin-start and margin below it blows away margin-start <dael> emilio: It's about setting, not getting. <dael> emilio: I filed a bunch of issues about serialization not round tripping. I'm pretty sure a shorthand that expands to multiple prop is a new problem <RRSAgent> I have made the request to generate https://www.w3.org/2018/11/14-css-minutes.html tantek <dael> dbaron: Example of something where behavior now isn't spec: If we assume that we haven't intro logical. If stylesheet says overflow-x:visitible and overflow-y:visible if you call on overflow you get visible <dael> dbaron: If you have overflow-x:visitible and overflow-y:visible overflow-block:visitible and overflow-start:visible what do you get? <RRSAgent> I'm logging. I don't understand 'make minutes public', tantek. Try /msg RRSAgent help <emilio> dbaron++ <dael> dbaron: Point isn't what answers are it's that answers aren't obvious in spec now <dael> TabAtkins: So it's legacy behavior running into this. I missed that. <dael> dbaron: I don't think it's legacy. I don't know any impl that has impl both logicila and physical shorthands. <dael> TabAtkins: Blink has logical of block <dael> dbaron: Longahnds, but shorthands aren't mapped. I think that's b/c there's so many spec issues in OM <dbaron> s/call on/call getPropertyValue on/ <dael> emilio: Have logical long hands and shorthands, but they only map to logical long hands <dael> TabAtkins: In cases right now where single property has both logical and physical underneath it only maps...I think we wanted the logical longhand to be ignored from shorthand after <dael> TabAtkins: While there are legacy issues with overflow in particular, the issue in general applies to all of our logical and physical properties. <dael> TabAtkins: I completely agree let's define that <tantek> regrets+ <RRSAgent> I have made the request to generate https://www.w3.org/2018/11/14-css-minutes.html tantek <dael> florian: majidvp experiment may provide feedback |
Per w3c/csswg-drafts#2988 the overflow shorthand should be parsed in logical order (block then inline) instead of physical order (x then y). Updated web platform tests. Bug: 872356 Change-Id: I674f8a42b2712e7d8fdfa4ba4a7575ed2fb7bc85
Any news here on the experiment/data? We still got x/y in the wild and just hoping for a shorthand similar to |
So, wait, this means |
It seems that it's too late to change I think the follow-up question is, can we / should we switch the order of Fwiw, the other 2-value properties that are following the "block inline" order are: |
Unless we want to resolve that we're happy with the status quo, we probably could do with a use counter on scroll-snap-align, to see if it's even possible to change it. |
Do you need a use counter beyond https://chromestatus.com/metrics/css/popularity#scroll-snap-align ? 4.6% seems to suggest there's a pretty big chance it's not changeable. |
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: [css-overflow-3] 'overflow' 2-value syntax is in wrong order<dael> github: https://github.com//issues/2988 <dael> astearns: We talked about this once <dael> tab <dael> TabAtkins: Thread conclusion is it was too late to change to block inline ordering. Stick to horz vertical <dael> TabAtkins: Designed scroll snap align to match. The question, then, is if we can or should switch scroll-snap-align to match or keep with other logical properties <dael> TabAtkins: fantasai said in issue we have two properties using block already. I suspect right answer is overflow is an unfortunate legacy and leave s-s-a to line up with the logical and suspect other values will be logical in future <dael> TabAtkins: My preference is s-s-a and future 2 value directional shorthands are logical <dael> astearns: Comment from David about s-s-a- usage being relatively high <heycam> q+ <dael> TabAtkins: I was saying we should not change <dael> TabAtkins: Keep all the current properties logical and align s-s-a with those <astearns> ack heycam <dael> heycam: A long time ago there were proposals to have keywords to opt into logical. Would that smooth over the edges of properties we can't change so authors can stay in logical if they stick the keyword in <dael> TabAtkins: Very possibly. I'm not deep into lore of why we haven't gone with that. Hvae to ask editor on logical properties spec. Could be an encouragement for that model <dael> TabAtkins: I suggest we resolve s-s-a and future 2-axis properties use the logical model <dael> astearns: Looking back to see what we resolved originally <dael> astearns: Logical is what s-s-a uses currently, yes? <dael> TabAtkins: Yes <dael> astearns: Any more discussion about the proposal? <dael> astearns: Prop: s-s-a and future 2-axis properties use the logical model <dael> astearns: This could be ameliorated by a switch to logical <dael> astearns: Obj? <dael> RESOLVED: scroll-snap-align and future 2-axis properties use the logical model |
Should this issue be closed with no change? |
Currently the 'overflow' 2-value syntax is parsed to be horizontal-vertical (to be precise, the first value maps to overflow-x, the second to overflow-y).
When possible, for any new 2-axis properties we're trying to map them to the logical longhands instead, so this is currently wrong. As well, for horizontal writing modes, the x/y ordering is opposite the logical ordering we've settled on, which is block/inline (to match what 4-value syntaxes do when you specify only two values).
As the two-value syntax is new, this should still be fixable - two values should map to overflow in the block and inline axises, in that order.
The text was updated successfully, but these errors were encountered: