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-backgrounds-4] Add background-repeat-x/y #116

Open
mstange opened this issue May 16, 2016 · 27 comments
Open

[css-backgrounds-4] Add background-repeat-x/y #116

mstange opened this issue May 16, 2016 · 27 comments

Comments

@mstange
Copy link

mstange commented May 16, 2016

It looks like recent versions of Edge and Chrome support the properties background-repeat-x and background-repeat-y. I haven't checked how far back support goes. Gecko does not support them, and Webkit doesn't appear to either.
If this is something people want, then it should be added to the spec.

@yisibl
Copy link
Contributor

yisibl commented May 25, 2016

Agree, I think it should be consistent with background-position-x/y.

@fantasai
Copy link
Collaborator

What's the use case for cascading them independently?

@SebastianZ
Copy link
Contributor

SebastianZ commented May 25, 2016

Backgrounds and Borders Level 4 already defines those two properties plus background-position-inline and background-position-block. So, I'm wondering why this issue was opened.

Gecko does not support them, …

That info is outdated. They were implemented in Gecko 49.

… and Webkit doesn't appear to either.

I don't have the means to test it, but the related MDN articles suggest that WebKit does support background-position-x and background-position-y. Could someone please verify that and update the articles accordingly (or let me know about it, so I can do that)?

Sebastian

@mstange
Copy link
Author

mstange commented May 25, 2016

Sebastian, this is about background-repeat, not background-position. (Also, I'm the person who implemented background-position-x/y in Gecko.)

Fantasai, I don't have a use case. Maybe it's just about consistency with background-position? To me, background-repeat "feels" like a shorthand property because it allows you to specify different values for each axis.

@fantasai
Copy link
Collaborator

If there's no use case for cascading them independently, then we should not add longhands. And Chrome and Edge should remove them.

@tabatkins
Copy link
Member

Our usage numbers show it at .05%, above our standard removal threshold.

@SebastianZ
Copy link
Contributor

Sebastian, this is about background-repeat, not background-position. (Also, I'm the person who implemented background-position-x/y in Gecko.)

Oops! Please forgive me my blindness! I saw you've also already filed bug 1273244 for it.

Sebastian

@dbaron
Copy link
Member

dbaron commented Jan 3, 2017

I wrote a few tests to allow testing which implementations support these.

@gregwhitworth
Copy link
Contributor

Updating here that Edge does not support the longhands.

@dbaron
Copy link
Member

dbaron commented Jan 12, 2017

It turns out that the group resolved to add these in April 2014.

(We also re-discussed them today, twice.)

@gregwhitworth
Copy link
Contributor

After discussing this after the resolution, I wasn't aware that deprecated implied that all other UAs that don't support this need to add this - my understanding of it's implication was more inline with obsolete. I would like to re-open this issue and discuss roughly defining it but marking it as obsolete. In addition to this, @FremyCompany looked into why Edge removed this and it was due to us implementing multiple backgrounds, and when testing Chrome sure enough you can't have multiple backgrounds in conjunction with background-repeat-x/y (https://jsfiddle.net/u5L8ak8b/2/ - I tried numerous vals to see if they mapped them together to no avail). By switching to obsolete, it allows us to vocalize to authors not to use it and allows Blink to, when possible take the necessary steps to remove it and doesn't burden others to implement it.

@kuoe0
Copy link

kuoe0 commented Jan 17, 2017

@gregwhitworth I think your test case missing a comma between two urls in background-image. And it's true that background-repeat-{x,y} can not accept multiple values. So there is only one repeat mode for this the two images in Blink with background-repeat-{x,y}. It means you can't create the same result created by the following source code with background-repeat-{x,y}.

background-repeat: repeat no-repeat, no-repeat repeat;

@gregwhitworth
Copy link
Contributor

@kuoe0 thanks for catching the missing comma. Even with that, it still seems odd to have a long hand that doesn't support that of the shorthand and so I still stand by allowing it in the spec and marked as obsolete, this allows Blink to, when they deem it possible, to remove it, but also doesn't imply that other UAs that don't support it to implement it.

@dbaron
Copy link
Member

dbaron commented Jan 19, 2017

So at the face-to-face meeting we discussed this once and then later in the afternoon discussed it again. Despite that we did reach a conclusion the second time, it seems that given Greg's #116 (comment) above, I think we need to discuss it again.

@dbaron dbaron added the Agenda+ label Jan 19, 2017
@tantek
Copy link
Member

tantek commented Jan 25, 2017

@gregwhitworth I should have said obsolete not deprecate at the f2f. I misspoke :(

@dbaron
Copy link
Member

dbaron commented Jan 25, 2017

We just discussed this again at the teleconference, ending with:

RESOLVED: Do not spec background-repeat-x/y for now

@SebastianZ
Copy link
Contributor

According to Google's data and the data from HTTP Archive, the use of background-repeat-x significantly increased over the past years and is now at 0.485%. The usage of background-repeat-y didn't increase as much but is still relatively high with 0.2%.

This indicates that there is some need for those longhands, even when there is only limited support by a single browser engine, at the moment.
Maybe someone familiar with querying HTTP Archive could find out for what pages are currently using those properties.

Sebastian

@gsnedders
Copy link
Member

There's some confusion above about whether WebKit does support them or not: it… sorta does—and Blink got it from WebKit. (This dates back to the original implementation of two-value background-repeat in WebKit.) The confusion likely arrises because we avoid exposing it… sometimes.

FWIW, I tried to remove these from WebKit as part of WebKit/WebKit@d70cd13 (given they were in many common cases not exposed)… and I've already had some breakage reported (because JS was assuming there were at least 10 longhand properties of background!).

Not made any decision about what we're going to do, but I strongly suspect we won't go back to the "semi-exposed" behaviour we had before, which means either totally shipping it or totally removing it.

See also https://bugs.chromium.org/p/chromium/issues/detail?id=1073573 for related oddness in Blink.

@SebastianZ
Copy link
Contributor

Thanks @gsnedders for the implementation insights!

Quoting my own comment

According to Google's data and the data from HTTP Archive, the use of background-repeat-x significantly increased...

This indicates that there is some need for those longhands...
Maybe someone familiar with querying HTTP Archive could find out for what pages are currently using those properties.

The goal behind this request was to get an answer for @fantasai's question back in 2016, i.e. to find out whether there are actually use cases for those two being defined independently.

I assume there is a use case for specifying no-repeat for one axis while keeping the other axis unchanged, i.e. repeat.
So defining background-image: url('pattern.png'); background-repeat-y: no-repeat; would result in a decorative strip. And an author wouldn't have to write background-repeat: repeat no-repeat;.

Skimming through the previous discussions, I still wonder what's the issue with those longhands that lead to the resolution to make them deprecated and lead implementers to remove them rather than specify and implement them properly.
(Note that the above is meant objectively. I don't have a strong opinion for or against adding them.)

Sebastian

@SebastianZ
Copy link
Contributor

Reiterating on my previous comments, the usage numbers of the two properties are steadily increasing. According to Google's data, we're now at ~0.513% and 0.241%, respectively.

This looks like a strong indicator that those longhands should be specified and implemented properly after all.

@gsnedders @dbaron Maybe one of you can answer the question why it was previously decided that they should be removed.

Sebastian

@SebastianZ SebastianZ changed the title [background-4] Add background-repeat-x/y [css-backgrounds-4] Add background-repeat-x/y Aug 11, 2023
@SebastianZ
Copy link
Contributor

Bringing this to the table again because the usage numbers are still increasing.

The proposal is to bring background-repeat-x and background-repeat-y back and properly define them.

Sebastian

@Loirooriol
Copy link
Contributor

Just want to note that splitting the property may make it hard (since we don't have a solution for #1282) to add new logical values like background-repeat: repeat-block. But of course, we might never add such values, or it will be fine when we solve #1282.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-backgrounds-4] Add background-repeat-x/y, and agreed to the following:

  • RESOLVED: add x,y,block,inline longhands, and the repeat-block and repeat-inline keywords to the shorthands
The full IRC log of that discussion <TabAtkins> SebastianZ: background-repeat-x/y as longhands for background-repeat
<TabAtkins> SebastianZ: Implemented in Gecko, usage numbers relatively high
<TabAtkins> SebastianZ: Usage numbers are around 0.5%
<TabAtkins> SebastianZ: So, want to specify this for web-compat
<TabAtkins> SebastianZ: Oriol pointed out it might make it hard to add logical keywords to background-repeat shorthand
<TabAtkins> SebastianZ: But I think we do need to do this as the usage numbers are high
<fantasai> TabAtkins: I agree, that level of usage means it's part of the Web, in effect
<TabAtkins> astearns: I haven't read thru entire backlog, but we resolved not to specify them *yet*
<TabAtkins> SebastianZ: At some point there was a resolution to not specify them, but couldn't find the reasoning behind it
<TabAtkins> SebastianZ: Probably because -x and -y are physical longhands?
<TabAtkins> fantasai: We have the same issue with bg-position, so we have to resolve it somehow
<TabAtkins> fantasai: So given usage numbers I think we should resolve to add -x, -y, -block, and -inline
<TabAtkins> fantasai: And then add repeat-block repeat-inline keywords
<TabAtkins> SebastianZ: Agree
<TabAtkins> +1
<TabAtkins> astearns: So proposed reoslution is we'll add x,y,block,inline longhands, and the repeat-block and repeat-inline keywords to the shorthands
<TabAtkins> astearns: Objections?
<TabAtkins> RESOLVED: add x,y,block,inline longhands, and the repeat-block and repeat-inline keywords to the shorthands

@Loirooriol
Copy link
Contributor

I'm opposed to both turning it into a shorthand and adding logical values with no plan about how to handle that. I'm fine with turning it into a shorthand without the logical values, or adding the logical values without turning it into a shorthand.

Same problem as #549 (comment)

@SebastianZ
Copy link
Contributor

@Loirooriol The situation here seems to be a different one. background-repeat already has physical based definition for its two-value syntax and supports repeat-x and repeat-y as keywords.
The logical keywords repeat-block and repeat-inline are added to complement the existing keywords and there isn't a conflict in var() usage or CSSOM.

Just in case we add a way to change how the two-value syntax should be interpreted, we run into the issues you refer to. Or is there something I am missing?

(Note that I turned background-repeat into a shorthand in #9756 and added the logical keywords for it now in #10755.)

Sebastian

@Loirooriol
Copy link
Contributor

So basically you want background-repeat: repeat-block to set background-repeat-x and background-repeat-y to an internal value that won't be resolved into used (why not computed?) value time?

I think people would generally expect it to expand into background-repeat-block: repeat; background-repeat-inline: no-repeat.

@cdoublev
Copy link
Collaborator

Is there any reason not to define a logical property group for background-repeat-* longhands? That is, I expect background-repeat to be defined as a Logical property group in the property definition table.

This would be needed to maintain relative order of declarations within a CSS declaration block, eg. with style.cssText = 'background-repeat: repeat; background-repeat-block: no-repeat; background-repeat-y: repeat'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Unslotted
Development

No branches or pull requests