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-ui] The computed value rules of user-select is problematic. #3344

Closed
emilio opened this issue Nov 25, 2018 · 3 comments
Closed

[css-ui] The computed value rules of user-select is problematic. #3344

emilio opened this issue Nov 25, 2018 · 3 comments
Assignees
Labels

Comments

@emilio
Copy link
Collaborator

emilio commented Nov 25, 2018

(Note: this is somewhat similar to #336)

I've been looking at user-select issues in https://bugzilla.mozilla.org/show_bug.cgi?id=1506547.

The rules for computing the value of user-select as specified in https://drafts.csswg.org/css-ui-4/#propdef-user-select have pretty problematic implications (which is why at least Gecko / Blink / WebKit don't implement it as such):

  • The initial computed value of the property depends on the element. If the element is editable the computed value should be contain, but otherwise it should be text. That is problematic since there's no canonical "initial computed style" anymore.

  • The computed value of a reset property depends on the parent style: This is problematic performance-wise. This is not unsolvable (justify-items has the same issue), but adds a lot of complexity and memory usage. This is worse for user-select since the common behavior is to "inherit" unlike justify-items. But basically this means that two elements that match the same rules no longer have the same computed style for reset properties, which is the invariant that powers some of Gecko's style engine optimizations.

Gecko / Blink / WebKit do all these checks at used-value time, but the initial value is auto. In Gecko the property is not inherited, but in Blink / WebKit it is inherited:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/css_properties.json5?l=3952&rcl=77578ccb4082ae20a9326d9e673225f1189ebb63
http://webkit.crisal.io/webkit/rev/324bfaa99696312d8e2daefbe60ae2ab0e616daf/Source/WebCore/css/CSSProperties.json#6250

Can we make these checks at used-value time instead, keeping auto as a computed value? You may need to walk the tree up anyway because of user-select: all, so this seems better.

We could implement this efficiently to some extent in Gecko by making it inherited and adding some hacks to set the default value when the parent style is contain and it's not overriden, or something of that sort, but I honestly don't think the complexity is worth it.

So concrete proposal would be:

  • Make user-select a non-inherited property (as it is today), with initial computed and specified value of auto.
  • auto means the following at used-value time:
    • If the element is an editable root, then the value is contain.
    • If there's no ancestor with a non-auto used value, or that auto used value is contain, then text.

Behavior-wise this is identical to the current spec, except auto is a valid computed value, and maybe related to display: contents depending if we choose to use ancestor box or ancestor element in the flat tree.

cc @frivoal @kojiishi @FremyCompany @rniwa

@emilio
Copy link
Collaborator Author

emilio commented Nov 25, 2018

cc @MatsPalmgren too :)

@frivoal
Copy link
Collaborator

frivoal commented Feb 25, 2019

Looks good to me. My intuition of what is easy to implement was the other way around, but you certainly know better. Happy to change if other implementers agree.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed The computed value rules of user-select is problematic, and agreed to the following:

  • RESOLVED: auto computes to auto, but the used value does what it currently says auto computes to
The full IRC log of that discussion <myles__> Topic: The computed value rules of user-select is problematic
<emilio> github: https://github.com//issues/3344
<myles__> emilio: user-select is messy. It’s inherited in some engines, but not in others. It has different initial values in some. I wanted to make it less insane, but the spec specifies that the initial value of the property si different depending on the element, which is not great.
<myles__> emilio: Depending on whether the element is editable or not, the initial value is different. I like when my initial styles are all shared.
<myles__> emilio: There is an unspecified value, but it computes to something at computed value times, was to support user-select:contain, IIRC
<myles__> emilio: I don’t want the computed value to be differ depending on which element the style applies to. We should do something closer to what Gecko does, which is close to WebKit and Blink. The initial value is auto, and once you specify it, that inherits down the tree
<myles__> emilio: Proposal is to make it non-inherited. Initial value is auto, and auto behaves differently depending on which element it applies to at used-value time
<myles__> florian: I was on an agreement rollercoaster
<myles__> florian: user-select:contain, only Edge exposes as a value, but all browsers have as a behavior. This is how selection works when you’re in an editable element. Selection can’t escape that element. All browsers do that. In Edge, that’s explained by the user-select:contain
<myles__> florian: Now that it’s a value, you can apply that value to other things.
<myles__> florian: Maybe you want to contain your selection to the body of content, so you don’t select UI
<myles__> florian: Maybe we need selective inheritance. All values inherit, except “contain”
<myles__> florian: Propagation would work this way. So the child of contain should be something else, but otherwise things inherit normally. At computed-value time this would be easier. If from an implementor point of view at used-value time is simpler, okay
<myles__> florian: It’s almost the same thing.
<myles__> florian: To support user-select:all, you might have to implement it anyway. The user difference is inexistant. Is it simpler in other engines?
<myles__> emilio: The current spec says “the property is reset in certain places, the property is not inherited, all values except contain effectively inherit, so you end up needing to change all engines to copy-on-write, which WebKit, blink, and Gecko do
<myles__> florian: it’s not inherited. If the value is auto, look at the parent.
<myles__> koji: Blink already experienced that once before, but used value is easier for us, too
<myles__> florian: okay
<myles__> emilio: Using this at computed value time means it breaks style system optimizations that assume non-inherited styles change, don’t need to restyle children.
<myles__> emilio: Also, given the initial value depends on the element it applies to, you no longer have a canonical initial value representation. Used value is better
<myles__> florian: Is it reasonable to assume webkit’s cooperation?
<myles__> astearns: proposed resolution: auto computes to auto, but the used value does what it currently says auto computes to
<myles__> dholbert: This needs to be reflected in the used value, too
<myles__> RESOLVED: auto computes to auto, but the used value does what it currently says auto computes to
<myles__> florian: There is some more magic here. Currently, on editable elements, the computed value is always contain, regardless. Should we keep it at computed value time or used?
<myles__> emilio: UA rule, so it can be override it.
<myles__> florian: We didn’t want it to be overridable
<myles__> emilio: Gecko allows it to be overridden. But it’s fine
<heycam> ScribeNick: heycam

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 20, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585

--HG--
extra : moz-landing-system : lando
mykmelez pushed a commit to mykmelez/gecko that referenced this issue May 21, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585
emilio added a commit to emilio/servo that referenced this issue May 25, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585
emilio added a commit to emilio/servo that referenced this issue May 25, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585
emilio added a commit to emilio/servo that referenced this issue May 29, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585

UltraBlame original commit: 807e0fabd519163758bcf5cfa63f7e7d2f1d9611
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585

UltraBlame original commit: 807e0fabd519163758bcf5cfa63f7e7d2f1d9611
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
The CSSWG decided that our behavior regarding inheritance is what we want,
see [1].

[1]: w3c/csswg-drafts#3344

Differential Revision: https://phabricator.services.mozilla.com/D11585

UltraBlame original commit: 807e0fabd519163758bcf5cfa63f7e7d2f1d9611
@frivoal frivoal self-assigned this Dec 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants