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

[css-overflow] Let 'overflow' accept two values #2484

Closed
Loirooriol opened this issue Mar 29, 2018 · 8 comments
Closed

[css-overflow] Let 'overflow' accept two values #2484

Loirooriol opened this issue Mar 29, 2018 · 8 comments

Comments

@Loirooriol
Copy link
Contributor

From #2000 (comment). overflow is a shorthand property of overflow-x and overflow-y, but it can only set them to the same value. It would be more convenient if the syntax was

[ visible | hidden | clip | scroll | auto ]{1,2}

The two values specify the behavior in the horizontal and vertical direction, respectively. If only one value is specified, the second value defaults to the same value. For example,

overflow: hidden auto; /* overflow-x: hidden; overflow-y: auto */

Then if #1282 introduces a flow-relative syntax for shorthands, it could apply to overflow, e.g.

overflow: hidden auto !relative; /* overflow-inline: hidden; overflow-block: auto */
@fantasai fantasai added the css-overflow-3 Current Work label Mar 29, 2018
@frivoal
Copy link
Collaborator

frivoal commented Mar 30, 2018

I don't know how urgent this is, but it seems reasonable to me.

@frivoal frivoal self-assigned this Apr 6, 2018
@fantasai
Copy link
Collaborator

fantasai commented Apr 9, 2018

Agreed with @frivoal’s comment, but there's the additional question of: should the ordering be physical or logical?

@Loirooriol
Copy link
Contributor Author

There is the precedent of e.g. background-position-x and background-position-y which are physically ordered in background-position. So if #1282 is going to add an easy way to switch shorthands to logical order, I prefer physical order for consistency.

@css-meeting-bot
Copy link
Member

The Working Group just discussed Let 'overflow' accept two values, and agreed to the following resolutions:

  • RESOLVED: Allow 2 values on the overflow property in physical x/y order in any existing values.
The full IRC log of that discussion <dael> Topic: Let 'overflow' accept two values
<fantasai> github: https://github.com//issues/2484
<dael> github: https://github.com//issues/2484
<dael> Oriel: It only lets you set overflow-x and overflow-y. It would be more convenient if it accepted two values. Then there was the question is the order should be physical or logical. As an example background-position is x and y. It will prserve physical order. There's another issue looking to switch longhand and shorthand into phsycial order.
<dael> florian: Other is issue #1282 which discussed adding logical keyword to all places currently phsycial.
<dael> astearns: Separate from that switch, do we let overflow accept 2 values?
<dael> astearns: Concern about changing this?
<dael> astearns: Weird mistyped decalrations may now have an effect?
<dael> florian: I suggest we presume that's rare and if it's a problem we raise it
<dael> florian: A more consistant system where they all have shorthands and they're physical.
<dael> astearns: Prop: Allow overflow to have two value and for the ordering to be physical.
<dael> iank_: Which order?
<dael> emilio: x and y.
<dael> dbaron: Quesiton: There are sets of values transofrmed into other values. If x is visible and y is scroll we make scroll into auto. Should those combos be syntatically valid for shorthand?
<dael> myles: Related that this shorthand shouldn't allow new functionality
<dael> dbaron: Transofrmation would still happen. I'm thinking a it would be nice if it rejected but b it's not possible because serialization problem. Because then values could not serialize to short hand
<dael> emilio: Happens in a lot of places.
<dael> dbaron: I guess it's not really a serialization problem. Do we want the things that are not going to be valid in the end be parse errors?
<dael> emilio: I think it would be weird if spec shorthand would yield different results.
<dael> myles: You set the shorthand and it's rejected and that's different that if you set the 2 longhands.
<dael> florian: You have a minifier and it takes the 2 longs and puts them to short and that's a parse error.
<dael> astearns: I would prefer let ou set the shorthand to whatever and letting it transform.
<dael> florian: I don't think we gain by forbidding these.
<dael> fantasai: If you serialize out computed values it's valid.
<dael> florian: What do we gain by forbidding?
<dael> dbaron: Reject things that don't makes sense.
<dael> florian: Makes sense when you transform.
<dael> dbaron: CSS ties to reject things that don't make sense.
<dael> fantasai: Would be nice if a validation pool flagged this.
<dbaron> s/ties/tries/
<dbaron> s/pool/tool/
<dael> astearns: Computed value shows something changed.
<dael> fantasai: That always happens.
<dael> emilio: [missed] fantasai Tranforming em to pixel doesn't show you've got a problem in your style sheet.
<dael> astearns: I'm not certain having a transofmr apply implies there's a problem in your stylesheet.
<dael> fantasai: rachelandrew?
<dael> rachelandrew: I don't have an opinion.
<dael> florian: Stylesheet maintenece it's strange.
<dael> myles: Have we never encountered this?
<dael> fantasai: Almost for display but we made all combos invalid. We got rid of the longhand.
<dael> emilio: Outline style stuff which when you have hidden outline and the line-width becomes 0.
<dael> astearns: Anyone have a concern with allowing whatever combo we spec? Anyone object to taking what we get and stransform?
<dael> astearns: Prop: Allow 2 values on the overflow property in physical x/y order in any existing values.
<dael> myles: What a combo authors want with different keywords?
<dael> astearns: hidden x auto in y.
<dael> myles: So only one scroller.
<dael> astearns: People use overflow x and y in various situations and it' sjust that it would be nice to let them use the shorthand for both.
<dael> rune_: If you have visible overflow in x and y you get visible.
<dael> florian: Transformed to auto.
<dael> myles: Hiddena nd auto are okay.
<dael> astearns: Obj
<dael> RESOLVED: Allow 2 values on the overflow property in physical x/y order in any existing values.

emilio added a commit to emilio/servo that referenced this issue Apr 12, 2018
Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2
bors-servo pushed a commit to servo/servo that referenced this issue Apr 12, 2018
style: Let overflow parse two values.

Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2
emilio added a commit to emilio/servo that referenced this issue Apr 12, 2018
Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2
bors-servo pushed a commit to servo/servo that referenced this issue Apr 12, 2018
style: Let overflow parse two values.

Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20627)
<!-- Reviewable:end -->
emilio added a commit to emilio/servo that referenced this issue Apr 12, 2018
Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2
bors-servo pushed a commit to servo/servo that referenced this issue Apr 12, 2018
style: Let overflow parse two values.

Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20627)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 12, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 12, 2018
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 12, 2018
Per w3c/csswg-drafts#2484.
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1453148
gecko-commit: eaf30a1f59eedf65d4d6a5009bebfd014873d64e
gecko-integration-branch: central
gecko-reviewers: xidorn
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 12, 2018
Per w3c/csswg-drafts#2484.
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1453148
gecko-commit: eaf30a1f59eedf65d4d6a5009bebfd014873d64e
gecko-integration-branch: central
gecko-reviewers: xidorn
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 15, 2018
w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
csnardi added a commit to csnardi/csswg-drafts that referenced this issue Apr 15, 2018
Resolves w3c#2484 by changing the grammar of overflow to accept two values, and identifying the order in which they are parsed.
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 16, 2018
w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 26, 2018
w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554078}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 26, 2018
w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554078}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 26, 2018
w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554078}
@CarterLi
Copy link

CarterLi commented May 1, 2018

Just noticed this in chromestatus

Can this new syntax allows `overflow-x: visible; overflow-y: hidden'?

eg. overflow: visible hidden won't be interpreted as overflow: auto hidden.

I think the old behavior was a design flaw and made no sense. Now it's a chance to (partially) fix the mistake, because it's a new syntax and won't cause compatibility issues.

https://stackoverflow.com/questions/6421966/css-overflow-x-visible-and-overflow-y-hidden-causing-scrollbar-issue#answer-6433475

@emilio
Copy link
Collaborator

emilio commented May 1, 2018

There's no change to the valid or invalid combinations of overflow-x/y.

@emilio
Copy link
Collaborator

emilio commented May 1, 2018

The reason for it, is because the normal overflow-x / overflow-y adjustment doesn't (can't) happen at parse time, so there's no way to differentiate this syntax from the other at that stage unfortunately.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 2, 2018
…and, a=testonly

Automatic update from web-platform-testsAccept two values in the overflow shorthand

w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554078}

--

wpt-commits: e6756de059f68d445306ac35a6b8e80633d75925
wpt-pr: 10477
fergald pushed a commit to fergald/csswg-drafts that referenced this issue May 7, 2018
Resolves w3c#2484 by changing the grammar of overflow to accept two values, and identifying the order in which they are parsed.
Moggers pushed a commit to Moggers/servo that referenced this issue Jun 13, 2018
Per w3c/csswg-drafts#2484.

Bug: 1453148
Reviewed-by: xidorn
MozReview-Commit-ID: D7M3PhnTtD2
@fantasai
Copy link
Collaborator

fantasai commented Jul 2, 2018

I'm a bit concerned that this is not consistent with scroll-snap-align, which is also two-valued, and is much more closely related to overflow than background-position is.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: 84fc374da5e3e01050664b3e3c34301a5eaea4c6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: eaf30a1f59eedf65d4d6a5009bebfd014873d64e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: 84fc374da5e3e01050664b3e3c34301a5eaea4c6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: eaf30a1f59eedf65d4d6a5009bebfd014873d64e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: 84fc374da5e3e01050664b3e3c34301a5eaea4c6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 2, 2019
Per w3c/csswg-drafts#2484.

MozReview-Commit-ID: D7M3PhnTtD2

UltraBlame original commit: eaf30a1f59eedf65d4d6a5009bebfd014873d64e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
…and, a=testonly

Automatic update from web-platform-testsAccept two values in the overflow shorthand

w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardichromium.org>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Cr-Commit-Position: refs/heads/master{#554078}

--

wpt-commits: e6756de059f68d445306ac35a6b8e80633d75925
wpt-pr: 10477

UltraBlame original commit: 22226423b006da14b61e173f36b6e228880d04db
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…and, a=testonly

Automatic update from web-platform-testsAccept two values in the overflow shorthand

w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardichromium.org>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Cr-Commit-Position: refs/heads/master{#554078}

--

wpt-commits: e6756de059f68d445306ac35a6b8e80633d75925
wpt-pr: 10477

UltraBlame original commit: 22226423b006da14b61e173f36b6e228880d04db
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…and, a=testonly

Automatic update from web-platform-testsAccept two values in the overflow shorthand

w3c/csswg-drafts#2484 details the resolution
by the CSSWG to accept two values in the overflow shorthand. Update our
implementation to match this, and also update two existing CSSOM
serialization tests. Additionally remove tests that are duplicates of
those currently found in WPT.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/4qF8XPQ1z2s

Bug: 833105
Change-Id: Id8f61182a7d7369a2f575acfdbf608600d1218dd
Reviewed-on: https://chromium-review.googlesource.com/1013618
Commit-Queue: Chris Nardi <cnardichromium.org>
Reviewed-by: Rune Lillesveen <futharkchromium.org>
Cr-Commit-Position: refs/heads/master{#554078}

--

wpt-commits: e6756de059f68d445306ac35a6b8e80633d75925
wpt-pr: 10477

UltraBlame original commit: 22226423b006da14b61e173f36b6e228880d04db
zdobersek pushed a commit to Igalia/webkit that referenced this issue Oct 9, 2019
https://bugs.webkit.org/show_bug.cgi?id=184691

Patch by Alexey Shvayka <shvaikalesh@gmail.com> on 2019-10-08
Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-overflow/overflow-shorthand-001-expected.txt:
* web-platform-tests/css/css-overflow/parsing/overflow-computed-expected.txt:
* web-platform-tests/css/css-overflow/parsing/overflow-valid-expected.txt:
* web-platform-tests/css/cssom/shorthand-values-expected.txt:

Source/WebCore:

In w3c/csswg-drafts#2484 it was resolved to accept one or two values in
the overflow shorthand, instead of only one. If two values are specified, the first would be used
for overflow-x and the second for overflow-y. This change was shipped in Firefox 61 and Chrome 68.
This patch implements new syntax while preserving handling of -webkit-paged-x and -webkit-paged-y.

Tests: fast/css/cssText-shorthand.html
       fast/css/getComputedStyle/getComputedStyle-overflow.html
       imported/w3c/web-platform-tests/css/css-overflow/overflow-shorthand-001.html
       imported/w3c/web-platform-tests/css/css-overflow/parsing/overflow-computed.html
       imported/w3c/web-platform-tests/css/css-overflow/parsing/overflow-valid.html
       imported/w3c/web-platform-tests/css/cssom/shorthand-values.html

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
* css/CSSProperties.json:
* css/StyleProperties.cpp:
(WebCore::StyleProperties::getPropertyValue const):
* css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeOverflowShorthand):
(WebCore::CSSPropertyParser::parseShorthand):
* css/parser/CSSPropertyParser.h:

LayoutTests:

* fast/css/cssText-shorthand-expected.txt:
* fast/css/getComputedStyle/getComputedStyle-overflow-expected.txt:
* fast/css/getComputedStyle/getComputedStyle-overflow.html:
* platform/ios/fast/css/invalidation-errors-2-expected.txt:
* platform/ios/fast/css/invalidation-errors-expected.txt:
* platform/mac/fast/css/invalidation-errors-2-expected.txt:
* platform/mac/fast/css/invalidation-errors-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250849 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants