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-3] Audit details of break-spaces value against use cases #2465

Closed
fantasai opened this Issue Mar 22, 2018 · 11 comments

Comments

Projects
None yet
6 participants

@fantasai fantasai added the css-text-3 label Mar 22, 2018

@frivoal frivoal self-assigned this Mar 26, 2018

@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Mar 26, 2018

On the one hand, I agree that over all, the various properties that affect line breaking and space preservation are a complicated mess from a UI point of view, and are hardly intuitive. It can be explained why which does what, but that's not the same thing.

Within that, I think overflow-wrap: break-spaces is not that unusual. It is not terribly obvious why that's there, but it can work, and it can be explained: The property is to deal with wrapping when the line would overwise overflow. Spaces at the end of the line are defined to overflow as "hanging" (which is either just ink overflow, or scrollable overflow, see #2398). Therefore, dealing with it there is OK.

Arguably, the value could also be attached to the word-break (e.g. a series of preserved white spaces is some kind of special "word") or line-break (e.g. this affects how the line breaks) properties. I suppose that if we did so, the new grammar should be [ currently accepted values ] || break-spaces. I am not theoretically opposed to that, but I am not convinced it is a real improvement either. It is just as easy to justify the value not being on these properties: word-break is self described as being about "soft wrap opportunities between letters", and line-break says it's "particularly how wrapping interacts with punctuation and symbols". At one point we also argued that it should be a value on the white-space property. I see no end to this discussion if we keep on reopening it.

Anyway, I am not objecting to better property design if there is a better solution, but I think that most likely, the better thing to do is to give it a rest, keep the current design we landed on, let people implement this (at least igalia is about to, and comments from others suggest that it may be in other people's roadmap as well). We may have a better chance at a sane UX by trying for nicer shorthand syntax in level 4, to paper over the mess that these various properties are in general, not limited to this question.

@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Mar 28, 2018

@litherum I believe you were looking into implementing this. Any opinion?

@rachelandrew

This comment has been minimized.

Copy link
Contributor

rachelandrew commented Apr 11, 2018

I was asked to have a look at this as someone who teaches this stuff. I'm not completely familiar with the whole spec but have read the linked meeting minutes. From an initial read I think I have a reasonable preference for how it is currently specified (as the overflow-wrap property). I think it better describes the situation of overflowing whitespace, rather than treating the preserved white space as a special word and therefore part of word-break.

I think I'd describe this as "overflowing whitespace", and I think that would be something any author would understand. Making the property seem sensible.

@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Apr 12, 2018

Right, that makes sense to me. So I think we should not change this syntax.

What we do need to tweak is that when overflow-wrap: break-spaces is on, the spaces are treated as more substantive things that otherwise, and therefore the one space at the end of the line that does not get wrapped should be counted towards in min-content inline size

@css-meeting-bot

This comment has been minimized.

Copy link
Member

css-meeting-bot commented Apr 12, 2018

The Working Group just discussed Audit details of break-spaces value against use cases, and agreed to the following resolutions:

  • RESOLVED: Spaces don't hang when break-spaces is specified
The full IRC log of that discussion <dael_> Topic: Audit details of break-spaces value against use cases
<dael_> github: https://github.com//issues/2465
<dael_> astearns: These are things we're rounding back to.
<dael_> astearns: This issue needs to be address to unblock something
<dael_> flackr: Igalia is about to impl and needs this closed.
<dael_> fantasai: There's several slight details. This is about break-spaces. Goal on some property is thre's a mode where editing says when you're tyuping a space
<dael_> fantasai: We want carat to move forward, spaces to not overflow as long as it' smore then 1em wide box so spaces don't hang.
<dael_> fantasai: There's currently a break spaces value on overflow wrap, but right now spaces are defined as can hang at end of line.
<dael_> fantasai: There's side effects of this. When you have a word at the end of the line and it fits but the space doesn't, even though you're breaking spaces it doens't move to next line but it does overflow. If you want things to wrap it's a problem this space overflows.
<dael_> fantasai: Related issue is that when you're trying to define intrinisic size of this box if the space is hanging it's not counted but if you want it visible you want to count it.
<dael_> fantasai: Seems to me if we want the space to behave like normal visible character, it should also effect size and not be hanging.
<dael_> fantasai: This means if the word fits and you type a space if the word fits it will wrap and we measure the space when measuring intrinisic size
<dael_> frWe fixed an issue in edge when people expect content:editible and not to wrap at same places because they use it for underlines. They use it as a rendering of the text.
<dael_> florian: So rendering should be same either way even if the reason is because when it's editable you wouldn't want anything else.
<dael_> florian: We're not proposing a difference, we're proposing both modes are same but when you're in editing the new behavior is sane, not almost sane.
<dael_> fremy: WE did the reverse.
<dael_> fremy: We made it so that when layout not taking space
<dael_> fantasai: fremy fix where you made them match that's good and if they want spaces to show and say break-spaces the spaces take up space.
<dael_> florian: But why align editable and not to not take space.
<dael_> fremy: Most content web is spaces not so we changed to that case.
<dael_> florian: It's hanging but it's there.
<dael_> fantasai: Edge behavior is in the spec now as most recommended as I recell
<dael_> fantasai: Change is that spaces don't hang when break-spaces is specified.
<dael_> florian: Rest is consiquences of that.
<dael_> fremy: Sounds good.
<fantasai> s/consiq/conseq/
<dael_> astearns: Obj to Spaces don't hang when break-spaces is specified?
<dael_> RESOLVED: Spaces don't hang when break-spaces is specified

frivoal added a commit to frivoal/csswg-drafts that referenced this issue Apr 25, 2018

@tabatkins tabatkins added the ready label Apr 25, 2018

frivoal added a commit to frivoal/web-platform-tests that referenced this issue Apr 25, 2018

frivoal added a commit to frivoal/web-platform-tests that referenced this issue Apr 25, 2018

@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Apr 25, 2018

@fantasai I've made a Pull request for the spec and one for the tests. Review appreciated.

frivoal added a commit that referenced this issue May 23, 2018

[css-text-3] Spaces don't hang when break-spaces is specified (#2612)
[css-text-3] Spaces don't hang when break-spaces is specified

Closes #2465
@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Jun 26, 2018

So, Igalia+Google are in the middle of implementing this, and have suggested we make a change to the syntax: instead of having break-spaces be a || alternative in overflow-wrap, it could be a | alternative in white-space.

Pros:

  • || alternatives are easier to implement than | alternatives
  • break-spaces is only useful when combined with white-space:pre-wrap, so you have to check that property anyway. It could instead be a variant of white-space:pre-wrap, on the same property.
  • There is not a particularly strong correlation between the break-spaces and break-word values from the point of view of where authors want to set them. They don't particularly need to be cascading together
  • It fits some people's mental model better

Cons:

  • in a browser that does not support break-spaces, the fallback of white-space: pre-wrap; overflow-wrap:break-spaces would be white-space:pre-wrap; overflow-wrap: normal. This is closer to the original intent than falling back from white-space: break-spaces to white-space: normal. This is can be easily mitigated by authors explicitly providing a fallback value (white-space: pre-wrap; white-space: break-spaces;) though.
  • It fits some other people's mental model better

The set of property and values that control line breaking and white space is messy and hard to understand anyway, and I don't think we can find a perfectly elegant answer. We've been renaming and shuffling this value between various properties for the last 3 years or so. Chasing perfect is the enemy of good.

Personally, I think either alternative is OK. Given that this request comes from the first implementation effort, I am inclined to support it, so that we can ship this asap, and finally put it in the hands of authors.

If the WG is firmly against that change, I'm OK with that too, and hope that Igalia+Google can live with that.

Either way, we should resolve and move on.

CCing people who have previously opined: @javifernandez @kojiishi @fantasai @litherum @tabatkins @rachelandrew

@frivoal frivoal reopened this Jun 26, 2018

@frivoal frivoal added the Agenda+ label Jun 26, 2018

@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Jun 27, 2018

Reposting a comment @fantasai accidentally made in the wrong issue:


Wanted to add some background: the proposal comes out of a discussion between me and Koji about the implementation of overflow-wrap: break-spaces.

The current syntax is overflow-wrap: break-word || break-spaces and Koji asked about splitting overflow-wrap into two longhands to simplify implementation. This was my response, which I'd advise everyone (regardless of interest in this particular property) to read since it's about principles that inform our decisions about property design.

I think that adding properties is more overhead on the authors—there is more to learn and memorize—so while it might be a little more awkward for implementors, it's better for CSS overall to avoid adding properties when they aren't necessary, for the authors' benefit.

(We made a mistake in Flexbox by splitting some of the properties for implementors' convenience, and now authors frequently get themselves in trouble by using longhands when they needed the shorthands to perform appropriate resets.)

For authors, the reason to have separate properties is if it's needed to cascade the settings independently. If I think about that, then I would say that break-word and break-spaces are not very closely related, and probably should cascade independently.

However, this makes me think also that white-space: pre-wrap and overflow-wrap: break-spaces should not cascade independently, so maybe it makes the most sense to shift the break-spaces value to the white-space property, e.g.

white-space: normal | nowrap | pre | pre-lines | pre-wrap | break-spaces

These are all mutually exclusive options on white-space: being able to combine break-spaces with any other value than pre-wrap is impossible, so the adjusted syntax reflects that structural relationship.

And this solves your implementation concern as well.

@kojiishi

This comment has been minimized.

Copy link
Contributor

kojiishi commented Jun 27, 2018

It might be clear already but I support the proposal. This proposal can help us reducing the necessary code change by ~100 lines for us.

@css-meeting-bot

This comment has been minimized.

Copy link
Member

css-meeting-bot commented Jun 27, 2018

The Working Group just discussed Audit details of break-spaces value against use cases, and agreed to the following:

  • RESOLVED: Accept the proposal in https://github.com/w3c/csswg-drafts/issues/2465#issuecomment-400171764
The full IRC log of that discussion <dael> Topic: Audit details of break-spaces value against use cases
<dael> github: https://github.com//issues/2465#issuecomment-400171764
<dael> florian: Reopening.
<dael> florian: We discussed for 3 years having a value called break-space on the overflow. Chrome and Igalia are mid-impl. Current syntax is not ideal. Would like to change it. I'm not enthusiastic, but their proposal is as good as what we've written and it's to impl so I'm in favor
<dael> fantasai: Proposal came from discussion with me and koji where koji said prop that takes multi values is harder then one value. Requested long hands so one can take break-word and the other break-space. Reason for sep prop is if they need to cascade sep. That does seem to be the case for these values. I can see why break-spaces and break-rulew ould be different
<dael> fantasai: Also notices break-spaces is only ever set with pre-wrap on whitespace. You'll never cascade independantly. Makes sense to move this from overflow-wrap to whitespace prop and then it would be mutually exclusive keywords
<dael> fantasai: That's how got ot proposal
<dael> fantasai: Improvement to cascade behavior. We're cascade independantly things that can be independant. I'm in favor of proposal
<dael> florian: So am I, koji , and Javier
<fantasai> s/independent/and not things which can't/
<dael> Rossen_: Objections?
<dael> RESOLVED: Accept the proposal in https://github.com//issues/2465#issuecomment-400171764
@frivoal

This comment has been minimized.

Copy link
Collaborator

frivoal commented Jun 28, 2018

@fantasai spec and tests PR done. Review appreciated.

frivoal added a commit to frivoal/web-platform-tests that referenced this issue Dec 26, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 5, 2019

Bug 1456739 [wpt PR 10627] - [css-text] white-space and intrinsic siz…
…ing, a=testonly

Automatic update from web-platform-tests
[css-text] overflow-wrap and intrinsic sizing

Goes together with the spec changes for w3c/csswg-drafts#2465

--
[css-text] break-spaces has moved from overflow-wrap to white-space

--
Merge pull request #10627 from frivoal/text-2465

[css-text] white-space and intrinsic sizing
--

wpt-commits: b439b5793f217eeb5ec1dd941161f6206ab2938d, ae2ef2d8cf5cab2db0a477411ea7f9cc79f6f737, 67b751ef3b96755111d9f845efd3b21c014d68bc
wpt-pr: 10627

mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 6, 2019

Bug 1456739 [wpt PR 10627] - [css-text] white-space and intrinsic siz…
…ing, a=testonly

Automatic update from web-platform-tests
[css-text] overflow-wrap and intrinsic sizing

Goes together with the spec changes for w3c/csswg-drafts#2465

--
[css-text] break-spaces has moved from overflow-wrap to white-space

--
Merge pull request #10627 from frivoal/text-2465

[css-text] white-space and intrinsic sizing
--

wpt-commits: b439b5793f217eeb5ec1dd941161f6206ab2938d, ae2ef2d8cf5cab2db0a477411ea7f9cc79f6f737, 67b751ef3b96755111d9f845efd3b21c014d68bc
wpt-pr: 10627

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 7, 2019

Bug 1456739 [wpt PR 10627] - [css-text] white-space and intrinsic siz…
…ing, a=testonly

Automatic update from web-platform-tests
[css-text] overflow-wrap and intrinsic sizing

Goes together with the spec changes for w3c/csswg-drafts#2465

--
[css-text] break-spaces has moved from overflow-wrap to white-space

--
Merge pull request #10627 from frivoal/text-2465

[css-text] white-space and intrinsic sizing
--

wpt-commits: b439b5793f217eeb5ec1dd941161f6206ab2938d, ae2ef2d8cf5cab2db0a477411ea7f9cc79f6f737, 67b751ef3b96755111d9f845efd3b21c014d68bc
wpt-pr: 10627

mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 8, 2019

Bug 1456739 [wpt PR 10627] - [css-text] white-space and intrinsic siz…
…ing, a=testonly

Automatic update from web-platform-tests
[css-text] overflow-wrap and intrinsic sizing

Goes together with the spec changes for w3c/csswg-drafts#2465

--
[css-text] break-spaces has moved from overflow-wrap to white-space

--
Merge pull request #10627 from frivoal/text-2465

[css-text] white-space and intrinsic sizing
--

wpt-commits: b439b5793f217eeb5ec1dd941161f6206ab2938d, ae2ef2d8cf5cab2db0a477411ea7f9cc79f6f737, 67b751ef3b96755111d9f845efd3b21c014d68bc
wpt-pr: 10627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.