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-text-decor] selective toggling in the text-decoration-skip property. #843

Closed
frivoal opened this issue Dec 28, 2016 · 8 comments
Closed
Labels

Comments

@frivoal
Copy link
Collaborator

frivoal commented Dec 28, 2016

Following up from #727 (comment), as that was a side track to the original issue.


Essentially, text-decoration-skip is a list of independent toggles, but you cannot easily add or remove one while keeping whatever else would be there intact.

Say I want to add leading-spaces and trailing-spaces to the default behavior, and put this in my author stylesheet:

:root { text-decoration-skip: objects leading-spaces trailing-spaces; }

Bam, I've just accidentally killed safari's by-default ink-skipping that I wasn't even aware of.

Or say I go the other way around, and like safari's by-default ink-skipping and add it to my author stylesheet to do the same in other browsers. Now if/when browsers add leading-spaces and trailing-spaces to the default value, I miss out.

Similarly, say I want to remove object-skipping for a particular element, <del> for example. How do I do that if I don't want to affect whatever else was being skipped, whether it came from default values, UA styles, or some other style I wrote myself?

I think the only precedent we have for this is the font-variant property, which is also a list of switches. If we apply the same model to text-decoration-skip, we'd have something like this (verbosity and bikesheding aside):

longhands:

text-decoration-skip-objects: none | objects
text-decoration-skip-spaces: none | [leading-spaces || trailing-spaces] | spaces
text-decoration-skip-ink: none | ink
text-decoration-skip-edges: none | edges
text-decoration-skip-box-decoration: none | box-decoration

shorthand:

text-decoration-skip: none | auto |
                     [
                       objects ||
                       [ spaces | [ leading-spaces || trailing-spaces ] ||
                       ink ||
                       edges ||
                       box-decoration
                     ]

none sets all longhands to none, auto is the initial value, and sets longhands to something smart (object ink leading-spaces trailing-spaces?), and other values are passed to through to the corresponding longhands and set all others longhands to none.

Alternatively, since we have no compat baggage to worry about and we would want authors to use these longhands to avoid the traps I mentioned at the beginning, maybe the shorthand should only have none and auto.
Actually I think I'd prefer that. Here's a possible bikesheeding:

text-decoration-skip: none | auto

text-decoration-skip-objects: none | all
text-decoration-skip-spaces: none | [leading || trailing] | all
text-decoration-skip-ink: none | all
text-decoration-skip-edges: none | all
text-decoration-skip-box-decoration: none | all

I know that's a breaking change and the spec is already in CR, but if you all agree it's a good idea, I wouldn't feel bad about doing it given the lack of implementations.

PS: the [leading || trailing] part could only be in level 4, but the rest should go in level 3 if we want to do it at all.

@kojiishi
Copy link
Contributor

kojiishi commented Jan 3, 2017

Not only +1 to selective toggling, I think there is another possible problem in the current design.

Let's say authors set html { text-decoration-skip: ink; } to their pages today.

The spec says:

Level 4 of this specification defines additional values (leading-spaces and trailing-spaces), and uses them in the initial value of this property, meaning that by default they are skipped.

So L4 UA must render decorations to leading/trailing spaces only on such pages, correct? This is probably unexpected, breaking behavior.

IIUC with the current design, or even with selective design as long as we have shorthands, we can't add new values that are skipping by default without breaking existing pages.

WDYT?

@kojiishi
Copy link
Contributor

kojiishi commented Jan 3, 2017

/cc @drott

@frivoal
Copy link
Collaborator Author

frivoal commented Jan 7, 2017

So L4 UA must render decorations to leading/trailing spaces only on such pages, correct? This is probably unexpected, breaking behavior.

IIUC with the current design, or even with selective design as long as we have shorthands, we can't add new values that are skipping by default without breaking existing pages.

You're right, any new "skip-by-default", whether introduced now or later, with or without shorthand, would be a breaking change, since it would be a change in behavior. I don't think there's any way around that, the only thing we can do is decide if the breakage is a problematic one or a negligible one, and whether the benefit brought by the new skip-by-default is outweights that breakage or not.

In the case of leading and trailing space, the difference is noticeable if there are &nbsp;s or if white-space is pre or pre-wrap. In all the cases I can think of, skipping by default is an improvement, but that's subjective, and authors are infinitely creative. In terms of compat risk, I'll note that if there's no &nbsp; but white-space is pre or pre-wrap, Safari already skips trailing spaces (but not the leading one). This (small) gap in compatibility probably helps us (a little) pick which ever behavior we prefer.

If we switched to an "auto|none" shorthand, and move everything else to longhands, and made leading-spaces and trailing-spaces skipping the (non controlable) default in L3, we would avoid breakage between L4 and L3, but we'd still have to introduce the breaking change now.

@kojiishi
Copy link
Contributor

kojiishi commented Jan 8, 2017

Thank you for the confirmation. With that, and with the problem you mentioned, I have mild preference not to add the shorthand for now. We can add it whenever we want to, but it looks to me that the risks win over the benefit at this point, and unless I miss something, the benefit of the shorthand for this function isn't very high?

@frivoal
Copy link
Collaborator Author

frivoal commented Jan 8, 2017

@kojiishi I am not sure I understand what you mean. Do you mean you prefer to keep the spec as-is with only the text-decoration-skip shorthand, no text-decoration-skip-* longhands, or to change to add only the text-decoration-skip-* longhands, and remove the text-decoration-skip shorthand?

Well, there's two part to my suggestion.

  1. add a text-decoration-skip-* as longhands of text-decoration-skip.

  2. limit the shorthand to only none and auto, requiring authors to use longhands for everything else

If we only want to do 1, we can do it any time we want (but I would still argue that the sooner the better). If we want to do 2 as well, we need to do it before it gets significant usage.

If you'd like to only add the longhands and remove the shorthand (possibly adding it back latter), we would need to do it now, but I am not sure what the benefit is compared to 1+2.

I think limiting the shorthand to resets only (none and auto), and using the longhands to toggle on and off the various aspects of skipping will lead to more robust and DRY code. This includes avoiding accidental overrides of whatever the browser's default value is, but also makes it easier to avoid fighting with yourself or your colleagues when you want different types of skipping to cascade separately.

For example, something like this is pretty painful to do without longhands:

* { text-decoration-skip-ink: all }
:lang(ja,zh,ko) { text-decoration-skip-ink: none }
:lang(zh), a[href] { text-decoration-skip-edges: all; }
img { text-decoration-skip-objects: all; }
pre code { text-decoration-skip-spaces: leading trailing; }

If that's the preferred coding style (and I think it should be), having any other value than none and auto is mostly a footgun.

Also, while any change to the default behavior is a “breaking” one, regardless of syntax, some breakage may be minor and advantageous. For example, I am not sure we will do it, but it doesn't seem crazy to me that we would eventually decide that the initial value of text-decoration-skip-ink is neither on nor off, but some kind of script dependent auto. If we limit the shorthand to auto and none and require authors to use longhands instead, everyone who hasn't explicitely set ink skipping one way or another will benefit. If we require (because we didn't introduce longhands) or allow (because we introduce longhands, but leave the full syntax on the shorthand) authors to use the shorthand, effectively everyone who has used this property, including for changes unrelated to ink-skipping, will miss out.

@fantasai
Copy link
Collaborator

fantasai commented Jan 1, 2020

OK, I've drafted up the changes discussed here and resolved at a high level in https://lists.w3.org/Archives/Public/www-style/2017Feb/0049.html

@frivoal Let me know if these edits look OK, or if there's something that should be fixed before closing out this overaching issue. (There are a number of detail issues which I think we should address separately.)

@frivoal
Copy link
Collaborator Author

frivoal commented Jan 6, 2020

@fantasai Thanks, this looks great.

I do have a few remarks. If you think any of these is an obvious bugfix, please fold that in and tick it off below. After that, I'm happy to re-file the rest into separate issues so that we can track them individually. For now, I'm dumping it all here as the result of a one-pass review, but I do expect most of these to move to separate issue.

  • text-decoration-skip-self says “Applies to: all elements”, but its object value's definition says: “Skip this element [...] if it is an atomic inline”. I wonder if this should be changed to “Applies to: atomic inlines”, or if you also envision a value that that skips the element even if it is not an atomic inline? If so, we may need to rename the existing value.
  • text-decoration-skip-inset's auto has “The UA should [...]”. Are you sure this is a should? As much as it makes sense to leave the size of the inset UA defined (and possibly context dependent), I'm not sure what the benefit is to allow UAs to implement this on/off switch at the syntax level and yet have no effect.
  • The grammar of text-decoration-skip-spaces defines start and end, but the initial value and prose calls them leading and trailing. I don't strongly care which one we pick (and we can bikeshed that separately if need be, as noted in the inline issue), but the spec should be self-consistent.
edited: refiled this part as a separate issue in https://github.com//issues/4653 * [ ] Safari and Firefox both have built-in default behavior that's sort of like `text-decoration-skip-spaces: end`, but not quite the same as the one specified, and not quite the same as each other: * Safari skips _all_ preserved U+0020 (but not &nbsp;, or U+3000, or U+2007) on the end side if `white-space` is `pre-wrap`, but not `pre` or `break-spaces` * Firefox skips _overflowing_ preserved U+0020 (but not &nbsp;, or U+3000, or U+2007) on the end side if `white-space` is `pre-wrap`, but not `pre` or `break-spaces` I wonder if we need to make the `end` value (and maybe other values) smarter, or if we need an `auto` of some kind in addition, or if this is just historical accident and all UAs will align on the specified behavior of `end` * [ ] The spec says that the initial value of `text-decoration-skip-spaces` is `start end`, which is consistent with what level 3 said in prose. However, Chrome behaves as if the initial value was `none`, and Firefox / Safari behave as if the initial value was `magic-end` (see previous point) and don't skip leading spaces at all. Not sure if the spec should align with the browsers or the other way around, but the discrepancy is worth noting. We do have a [resolution in favor of what the spec says](https://lists.w3.org/Archives/Public/www-style/2015May/0314.html), so it's probably just a matter of writing tests and filing bugs…
edited: moved this part as to This has been moved to https://github.com//issues/4568 * [ ] I suspect some or all of these properties should also apply to the `::marker` pseudo.

@frivoal
Copy link
Collaborator Author

frivoal commented Jan 7, 2020

@fantasai I've applied the two tweaks you agreed about, and fixed a couple more bugs editorial / markup bugs along the way. Closing, as I'm happy with the resolution and its execution.

@frivoal frivoal closed this as completed Jan 7, 2020
@frivoal frivoal added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. Needs Testcase (WPT) and removed Commenter Response Pending labels Jan 7, 2020
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