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-rhythm-1] Avoiding accidental double spacing #938

Open
frivoal opened this Issue Jan 16, 2017 · 80 comments

Comments

Projects
None yet
@frivoal
Copy link
Collaborator

commented Jan 16, 2017

The spec for https://drafts.csswg.org/css-rhythm/#line-height-step says that

It is usually recommended to set the line-height lower than the step unit. The used line height can increase due to several factors such as the use of vertical-align or font fallback.

The initial value of line-height is normal which is defined like this:

normal
Tells user agents to set the used value to a "reasonable" value based on the font of the element. The value has the same meaning as <number>. We recommend a used value for 'normal' between 1.0 to 1.2. The computed value is 'normal'.

Actual implementations are know to vary, and in practice the actual numerical value of line-height: normal is unpredictable. Ideally authors will not rely on it and manually set the line-height when they want to use line-height-step, but we should try to be robust when they don't.

If normal in a particular browser on a particular os with a particular font is equivalent to 1.15, and on a different browser/os/font combination, it is equal to 1.25, and an author leaves line-height at the default value, and sets line-height-step to 1.2em, things will look fine in one browser and not in the other, and the author may not know if they do not test in that particular browser/os/font combination.

I think it is quite important to reduce or eliminate this kind of situation, so we should do something.

How about this:

When line line-height-step is not 0, then line-height: normal is treated as line-height: 1 when computing the used value. This would usually be appropriate (and in the odd case where it is not, it is always possible to set a manual value), and would eliminate variation between browsers, making naïve use of line-height-step more robust.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Jan 23, 2017

I understand the motivation, but I'm not a big fan of adding more automatic magic behaviors.

line-height: normal causing different layout by browsers/platforms are already widely known facts. Most recent reset.css has line-height: 1. Fixing it automatically only when line-height-step is set looks like we're recommending not to to set line-height. We're not, correct?

Also, the problem you state isn't limited to line-height. What if author forgot to set font-size? If the font-size is larger than a test device, it may result in two units, but we can't fix it, correct?

I have a mid preference to let authors to deal with their errors, and minimize the magic behaviors in the platform.

@kojiishi kojiishi added the Agenda+ label Jan 24, 2017

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 25, 2017

line-height:normal being a little different depending on the browser is not a major problem when step sizing is not involved, because the differences are subtle. In this case, the differences could be large (some browsers/os/fonts combinations getting double spaced, some other not), and the fix seems easy.

When the problems are obvious and the solutions hard, leaving things up to authors is fine. Here it's not the case.

@fantasai

This comment has been minimized.

Copy link
Collaborator

commented Jan 25, 2017

If we make behavior dependent on whether or not line-height-step is initial, the initial value needs to be none.

If we want font-based line heights to be possible to use with step-sizing, we need to add a normal keyword (that computes to its absolute length) to line-height step

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2017

If we want font-based line heights to be possible to use with step-sizing, we need to add a normal keyword (that computes to its absolute length) to line-height step

Can you clarify how author can do:

line-height: normal;
line-height-step: 18px;

that was what I wanted to ask but probably failed, sorry about that.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2017

If we make behavior dependent on whether or not line-height-step is initial, the initial value needs to be none.

I'm fine to add none if desired.

kojiishi added a commit that referenced this issue Jan 25, 2017

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2017

Added i18n label because this proposal will cause lines to overlap for some scripts.

kojiishi added a commit that referenced this issue Jan 26, 2017

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2017

Since I wasn't able to express my points very well in the weekly conf call, let me continue my points here by cc'ing people who spoke at the weekly on 25 Jan, from IRC log:
@astearns @stevezilles @tantek @tabatkins @dbaron @fantasai @dauwhe @litherum @FremyCompany
There were some points what I said didn't seem to be understood correctly, or I didn't understand even by reading IRC log, so appreciate to read through and comment here if any.

line-height: normal doesn't produce a robust layout as discussed, and I agree that a robust layout is one of important principle in CSS, and that line-height: normal is against that.

The purpose of line-height: normal is different; it is designed to avoid line overlaps as much as possible. Making any text readable without unexpected overlaps is also one of important principles in CSS. These two important principles conflict to each other. I think that is why line-height gives a choice to authors, and I think CSS chose the right default because overlapping is worse problem.

What I was trying to say at the weekly was that we need to give the choice to authors regardless rhythmic sizing is used or not, and IIUC this is also what @litherum was trying to say.

I made a multi-script sample text here (credits to @r12a and @aphillips, thank you!) For scripts in this example, I hope you see normal works perfectly in all browsers, but if we were to pick one fixed value, probably "1.8" is a good value. But I saw an example where "1.8" wasn't still tall enough before. Pick "2"? But it's too tall for some other scripts, such as Latin.

Or, maybe, I wonder, from @fantasai's comment above, this point was understood but cases where font metrics-based line height and rhythmic sizing used together was not understood?

If that's the case, I'm quite sure this case is common; author wants a constant rhythm, such as 18pt. The page has CGM that text can be in various scripts. Author wants to prioritize "lines not to overlap" over "possible additional space when falls back to bad fonts occurs," because font fallback is more predictable than CGM text content. In this case, the combination of normal and rhythmic sizing is what the author needs.

If the author uses line-height: normal, Latin lines will fit to one step unit, while Tibetan may fit to two or three step units, but still the rhythm is kept.

...or maybe concerns discussed there was different from either of above, appreciate if someone can explain more if so.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Feb 3, 2017

Talked with @tabatkins offline, he said he might try to come up with a new proposal, so I look forward to seeing it.

He also said it became clearer after we talked, so I guess I didn't do good job in conf call or comments above. Sorry about that, let me try to simplify the points.

  1. I understand the interop problem with line-height: normal.
  2. I understand there are cases where rhythmic sizing may make the issue (1) worse.
  3. I believe this proposal sacrifices a CSS readability feature for tall scripts.
  4. I think this proposal is net negative, given (3) looks more critical than (1) to me.
  5. I mildly prefer solving the original issue (1) altogether, not only the worse one (2).

Happy to discuss more if anyone can come up with new proposals, or disagree with me on 3-5.

@RByers

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2017

Blink is considering shipping line-height-step. Please let us know if there's a specific proposal here for changes which could cause a web compat problem in the future if we were to ship now. If approved, line-height-step support could make it to Chrome stable as early as June, so we'd still have a few months to safely make breaking changes or pull the feature if consensus can be reached.

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 7, 2017

Alright, I think I have a solution that lets line-height:normal fulfill its purpose and solves the double spacing problem we've been discussing.

Hypothesis:

  • line-height:normal needs to be left alone, for the reasons Koji said
  • When setting the line-heigh-step, you (almost?) never want to set it to something smaller than the line height of the element that establishes the rhythm.
  • When inheriting line-height-step, you do want it to be able to be smaller than the line height of the element it is inheriting into. That is what enables things like titles to be enlarged to a multiple of the step size.

Proposal:
line-height-step: none | [ <length> [ safe | unsafe ]? ]

  • none computes to (or behaves as, I don't care) 0 unsafe
  • if both safe and unsafe are omitted, it is the same as safe
  • <length> unsafe does the same as what the spec says now
  • <length> safe computes to max( 1lh, <length>) unsafe. Nb: lh resolves at computed value time to an absolute lenght, so the computed value of line-height-step, and therefore what we will inherit, will be an absolute length.

Let's walk through that:

<article>
<h1>Title!</h1>
Lorem Ipsum…
</article>
article {
  font-size: 16px;
  line-height: normal;
  line-height-step: 20px;
}
h1 { font-size: 1.5em; }

First, assume a font for which line-height: normal gives 1.2:

  • natural line height of article lines: 16px × 1.2 = 19.2px
  • natural line height of h1 lines: 16px × 1.5 × 1.2 = 28.8px
  • line-height-step on the article element: 20px safe ⇒ 19.2px < 20px ⇒ 20px unsafe
  • line-height-step on the h1 element: inherit ⇒ 20px unsafe
  • adjusted line height of article lines: 19.2px ≤ 20px ⇒ 20px
  • adjusted line height of article lines: 20px < 28.8px ≤ 2 × 20px ⇒ 2 × 20px = 40px

⇒ Same as now

Now, let's assume a font for which line-height: normal gives 1.3

  • natural line height of article lines: 16px × 1.3 = 20.8px
    ** This is accidental. The author did not anticipate that on some other OS which they do not test on, the default font is different and larger than the step size. With the current spec, this would result in double lines for every line of the article, which is not what the author intended.**
  • natural line height of h1 lines: 16px × 1.5 × 1.3 = 31.2px
  • line-height-step on the article element: 20px safe ⇒ 20.8px > 20px ⇒ 20.8px unsafe
  • line-height-step on the h1 element: inherit ⇒ 20.8px unsafe
  • adjusted line height of article lines: 20.8px ≤ 20.8px ⇒ 20.8px
  • adjusted line height of article lines: 20.8px < 31.2px ≤ 2 × 20.8px ⇒ 2 × 20.8px = 41.6px

⇒ The step size is a little larger than expected, allowing the rhythm to work without creating blank lines everywhere

@frivoal frivoal added the Agenda+ F2F label Apr 7, 2017

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2017

I don't understand what this means:

safe computes to max( 1lh, ) unsafe. Nb: lh resolves at computed value time to an absolute lenght, so the computed value of line-height-step, and therefore what we will inherit, will be an absolute length.

What is the computed value of max(1lh) when line-height: normal? I tried to learn what the lh spec says, but I can't read from it.

Could you write down in more plain English?

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2017

BTW, a discussion at mozilla.dev.platform clarified that the use cases (and thus desired behaviors) are different between mine and Latin. This has been gradually understood, if I think now, but I was unconscious. The discussion was so much helpful to clarify that.

So I added a section about East Asian Casual Vertical Rhythm to clarify different two (or three, not sure at this point) use cases, and which property and combination works good for which use cases.

I'm under impression that even for the strict vertical rhythm, Latin and East Asia are different enough that we're likely to end up with 3 different behaviors; 1) East Asian Casual, 2) East Asian Strict, and 3) Latin Strict, but maybe we can merge 2 and 3, or other writing systems have 4th or more use cases. That should be figured out sometime in future, but as I wrote there, I'm focusing on East Asian Casual first.

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 10, 2017

What is the computed value of max(1lh) when line-height: normal? I tried to learn what the lh spec says, but I can't read from it.

Could you write down in more plain English?

line-height:normal behaves the same as if a number was specified, except the author doesn't know what that number is, since it depends on the font (and on how the browser chooses to deal with it).
But once the browsers knows the font and the font size, it can know how many pixels tall a line is going to be when line-height is normal. That's 1lh. Of course the actual line might end up being taller if you change font size on inlines, or have inline images, or superscripts... But assuming a line with nothing special in it, you know how tall it would be.

So, if you have specify 20px safe, either:

  • that 20px is taller than what this natural line height would be, in which case, everything behaves exactly the same as you specified
  • or 20px is smaller, in which case it's rounded up to the whatever the size of the line would be, and then use & inherit that rounded up value.

I think that should cover 99% of the use cases (which is why I made "safe" the default), and if you actually want the unsafe behavior, you can ask for it.

@dbaron

This comment has been minimized.

Copy link
Member

commented Apr 10, 2017

I believe line-height: normal is allowed to do things that are a function of the font -- which means it can be a function of font selection and the actual text that is in the element. (I'm not sure if any UAs treat it as a function of the text, or if they just use the first available font. There was definitely discussion in the WG that it ought to consider all fonts used in the text.)

If it's a length unit, it needs a definition of 'normal' that doesn't depend on text.

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 10, 2017

I'm going after the CSS2.1 definition, which although phrased somewhat vaguely, does not seem to allow (by my reading) considering the actual content of the element:

normal
Tells user agents to set the used value to a "reasonable" value based on the font of the element. The value has the same meaning as . We recommend a used value for 'normal' between 1.0 to 1.2.

I'm sure we can find some wiggle room in that phrasing, but regardless, I would indeed favor a definition of 'normal' that doesn't depend on text. The alternative is just going to make everything more complicated.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2017

@frivoal, I'm so disappointed at my English skill, I still can't understand what the proposal is after tens of minutes of try. Could you try plainer English, or meta-code if possible?

When you said:

that 20px is taller than what this natural line height would be, in which case, everything behaves exactly the same as you specified

I interpreted:

  • 20px is the specified value of 'line-height-step'.
  • "natural line height" is the used line height from the layout result before rounding is applied?
  • "as you specified" means the specified value of 'line-height'?

so this means, in meta-code:

if (computed-line-height-step > line-height-from-layout-result) {
  if (specified-line-height is not "normal")
    use specified-line-height;
  else
    use ???
}

This logic doesn't make sense, so I'm probably lost somewhere, but don't know what it is.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2017

See 10.6.1 Inline, non-replaced elements

If more than one font is used (this could happen when glyphs are found in different fonts), the height of the content area is not defined by this specification. However, we suggest that the height is chosen such that the content area is just high enough for either (1) the em-boxes, or (2) the maximum ascenders and descenders, of all the fonts in the element. Note that this may be larger than any of the font sizes involved, depending on the baseline alignment of the fonts.

If UA chose to do (2), "all the fonts in the element" depends on text, and that's all 4 browsers do today when line-height: normal, as @dbaron pointed out.

@astearns astearns removed the Agenda+ F2F label Apr 18, 2017

@css-meeting-bot

This comment has been minimized.

Copy link
Member

commented Apr 20, 2017

The CSS Working Group just discussed Avoiding accidental double spacing.

The full IRC log of that discussion
<astearns> topic: Avoiding accidental double spacing
<astearns> github issue: https://github.com/w3c/csswg-drafts/issues/938
<astearns> github topic: https://github.com/w3c/csswg-drafts/issues/938
<rachelandrew> florian: this is about avoiding accidental double spacing, this might happen if an author sets line height step to 1.3 em, and the default font using line-height normal turns out bigger than 1.3 em
<rachelandrew> in another font the line-height might be calculated at double height
<rachelandrew> the previous proposal was to make the height deterministic
<rachelandrew> but koji pointed out that line-height normal is useful as it avoids overlap with long ascenders and descenders
<rachelandrew> florian: has a proposal for this
<rachelandrew> when you have something with a tall line-height you want it to be double, when it inherits this is a feature
<rachelandrew> when you set it, you never want to accidentally make the line-height step smaller than the natural line-eheight
<rachelandrew> soemtimes you want to do it on purpose
<rachelandrew> proposing to add a safe /unsafe keyword pair defaulting to safe
<rachelandrew> current behaviour is the unsafe one
<rachelandrew> dbaron: I understand but think it is a problem
<rachelandrew> the check is whether the line-height step is smaller than th eline-height and if so make the step bigger
<rachelandrew> the 2 pieces I am concerned about, 1 is that normal is a computed value of line-height
<rachelandrew> having that go back and influence line height step doesn't seem as if it will work
<rachelandrew> we do sometimes have font metrics influence computed values
<rachelandrew> florian: you need ot take th eline-height unit value as the lh unit depends on it
<rachelandrew> dbaron: so maybe that is ok, the other thing is that if you use text for more than one font we claim you end up creating a box of the size of the. line-height around both sets
<rachelandrew> creating a height that is larger than the line-height
<rachelandrew> fantasai: thats a general problem
<rachelandrew> dbaron: to the whiteboard
<rachelandrew> florian: does this change what line-height normal means
<fantasai>  dbaron draws a Chinese character and a Latin letter
<rachelandrew> dbaron: demonstrates lining up characters on the whiteboard
<fantasai> dbaron: suppose these come from different fonts
<fantasai> dbaron draws the ascent and descent on the chinese character
<fantasai> dbaron then draws the ascent and descent on the Latin letter, which is a little bit lower
<rachelandrew> florian: which is the primary font and which is the fallback
<rachelandrew> dbaron: it shouldn't matter as per the spec
<rachelandrew> fantasai: I think there is a conflict in the spec
<rachelandrew> dbaron: the number line-height is 1.26 font size 16px, so we want a 20 pixel box round each of these
<rachelandrew> dbaron demonstrates where the line-height boxes are around the two characters
<fantasai> (dbaron is using a numeric line height in place of normal, because normal could result in varying line heights; didn't want to deal with the complexity yet)
<rachelandrew> dbaron: end up with a line box height bigger than the line-height
<fantasai> dbaron notes that because the ascent plus descent plus half-leading of each adds up to 20px, but they are offset by 1px, the total line height is 21px
<fantasai> fantasai says the CSS2.1 spec says two contradictory things, one of which is that
<rachelandrew> florian: in this case if you started with a line-height step of 30 pixels you wouldn't get double spacing
<rachelandrew> dbaron: the designer might have fonts that have the same box and others have different boxes and get the double spacing
<fantasai> https://github.com/w3c/csswg-drafts/issues/391
<rachelandrew> florian: for it to fail that way you need multiple fonts falling back to each other
<rachelandrew> fantasai: I think this point is really important, it is a critical flaw in how it works
<rachelandrew> when you have boxes of the same height, the CSS2.1 spec says 2 different thinks
<rachelandrew> you get that result and also get 20 pixels out of that
<rachelandrew> so we need to fix that
<rachelandrew> in order for line-height-step to be at all robust and not flaky we need to fix it
<rachelandrew> myles: Gecko and spec agree with dbaron's example, WebKit would calculate the height and add spacing to either side to make it equal
<fantasai> (The spec says two contradictory things, and supports both Gecko and WebKit's interpretations depending on which sentence you read in the paragraph describing this)
<dbaron> I don't think that's actually what Gecko does
<dbaron> dbaron: oh, yeah, that is what the spec says -- add the half-leading after unioning the character heights
<rachelandrew> astearn: this is one crucial instance, but initially you were talking about making step sizing in the safe mode that would increase itself if it encountered a large line-height
<rachelandrew> florian: where you have set your step size accidentally smaller than the computed value of line-height
<rachelandrew> fantasai: can we resolve on fixing this?
<fantasai> https://github.com/w3c/csswg-drafts/issues/391
<rachelandrew> koji: how do you compute line-height normal to pixels
<rachelandrew> <general disagreement about the spec>
<rachelandrew> dbaron: it's more like a number than any other kind of value but it does a thing based on the font
<rachelandrew> myles_: it is based on the default font
<dbaron> Here's the thing I drew on the whiteboard: https://lists.w3.org/Archives/Public/www-archive/2017Apr/att-0006/IMG_20170420_102101.jpg
<dbaron> (although as myles__ pointed out, I was wrong)
<astearns> the point I wanted to make that you use step sizing to create vertical rhythm. Creating a 'safe' version that avoids double spacing and breaks the rhythm is counterproductive
<rachelandrew> florian: is there a definition for the used value of line-height, if you have normal and know the font
<rachelandrew> fantasai: at computed time the lh unit needs to map to an actual value
<dbaron> dbaron: I think you get it from the first available font
<rachelandrew> florian: the value of the line-height property does it change based on content on the line?
<rachelandrew> does that property vary per line?
<rachelandrew> myles_: no properties do not vary per line
<rachelandrew> florian: you can't change the primary font per line
<rachelandrew> dbaron: I think florian is talking about multiple lines splitting the same element
<rachelandrew> the computed value of line-height normal is normal
<rachelandrew> so if you are asking what lh does, when you have line-height normal use the first available font in the font list
<rachelandrew> florian: not only is the value normal, but the value of line-height 1.2 is 1.2
<rachelandrew> dbaron: let's file a spec issue
<rachelandrew> florian: what makes this impossible
<rachelandrew> dbaron: spec needs to say how to compute to an abs length, probably using the primary font
<rachelandrew> florian: do you need to do layout to figure out the pixel value of line-height?
<rachelandrew> dbaron: for the lh unit or for layout?
<rachelandrew> florian: not talking about the line box
<rachelandrew> what does the UA pick the number based on?
<rachelandrew> fantasai: I think you assume this gets computed by a number and used as that number for layout calcs, used as a length to figure out the leading
<rachelandrew> that conversion happens per font in the text
<rachelandrew> myles_: webkit does this
<rachelandrew> fantasai: if you don't have a value for the entire run, how can you work out the line-height for a single value
<dbaron> ok, filed https://github.com/w3c/csswg-drafts/issues/1253 on css-values-4
<rachelandrew> myles_: (answering q) we do a bunch of stuff for the width then we calculate heights and place vertically
<rachelandrew> fantasai: for each char you find ascent and descent, get a value, if it is not the line-height increase or decrease on both sides to get the line-height
<rachelandrew> dbaron: do you then use the normal?
<rachelandrew> dbaron: use line-height normal, a single element on the line, font fallback is happening in the line, fonts have different etrics for line-height normal and ascent and descent
<rachelandrew> ... you take the ascent and descent, where do you get the number for normal
<rachelandrew> myles_: we don't
<rachelandrew> dbaron: Gecko includes the external leading
<rachelandrew> florian: how is this not a violation of 2.1?
<rachelandrew> koji: this sentence contradicts other parts of the spec
<rachelandrew> dbaron: that sentence was written in the olden days
<fantasai> s/we don't/we don't; we take that value with no extra leading/
<rachelandrew> dbaron: it assumes defaulting to what the font says
<dbaron> https://drafts.csswg.org/css2/visudet.html#inline-non-replaced
<rachelandrew> koji: describes content box height
<rachelandrew> fantasai: the content box height has no effect on layout
<rachelandrew> dbaron: it matters re leading around it
<rachelandrew> fantasai: but this section is not related to line-height normal
<rachelandrew> dbaron: it can change the size of things, because the leading changes the content height, the middle of where the content height is does matter to layout
<rachelandrew> astearn: we're not really addressing the rhythm issue here, though we found bugs in other specs
<rachelandrew> florian: do we have an issue to define the line-height unit based on primary font
<rachelandrew> dbaron: pasted such an issue earlier
<rachelandrew> florian: the issue I proposed is to define the same way
<rachelandrew> koji: this is based on font metrics of the primary font in safe mode
<rachelandrew> florian: when it inherits, is specified. It doesn't depend on the font
<rachelandrew> astearns: you set your vertical rhythm for body text but the heading is bigger what happens
<rachelandrew> florian: the line step height will double to keep the rhythm
<rachelandrew> gets the unsafe value from the pixel
<rachelandrew> not magic, doing this based on a keyword, unsafe is current behaviour. safe will cause the bigger line-height behaviour
<rachelandrew> koji: does this change based on inheriting?
<rachelandrew> florian: no magic with inheriting
<dbaron> ok, filed https://github.com/w3c/csswg-drafts/issues/1254 on line-height:normal definition in CSS 2.1
<rachelandrew> florian: how well this works depends on how we fix 2.1, it doesn't make things worse
<rachelandrew> koji: I don't see it saves much, maybe some cases people think it is better
<rachelandrew> fantasai: keyword on line height setp to us line-height
<rachelandrew> astearns: that would make double spacing more likely
<rachelandrew> florian: you don't want to be exactly the line-height, needs to be just a bit more
<rachelandrew> astearns: step sizing could be a percentage of line-height
<rachelandrew> florian: either line-height is a number and works, or it isn't
<rachelandrew> if you want to set your step smaller than the line-height, set it explicitly with unsafe then it will not get adjusted up
<dbaron> Koji: on web, due to small screen real estate, people set grid to fraction (half) of the desired line height
<dbaron> astearns: that happens in print as well
<rachelandrew> koji: I understand some cases it helps, given complexity vs benefit
<rachelandrew> florian: it is very frustrating when people won't use a browser if it breaks, the outcome of not dealing with this could cause problems with different sized fonts, in one browser double heights might happen based on default sizing
<rachelandrew> fantasai: having a layout system that breaks badly when fonts get substtuted is bad
<rachelandrew> double spacing is bad
<rachelandrew> dbaron: authors should not have to write a number in their CSS that has to be right based on the font the user has
<rachelandrew> florian: the feature currently requires users pick a value
<rachelandrew> dbaron: the dev currently has to write line-height 3 or 20 pixels, Chrome might switch to double spacing based on one value, Firefox might pick another value, a subtle difference means you get double spacing in one browser and not another
<rachelandrew> dbaron: Gecko has had to reduce line-height they compute for webcompat issues
<rachelandrew> fantasai: we don't want more of these situations
<rachelandrew> astearns: should we come up with a way of making step sizing safer, we haven't come up with a resolution on that
@dbaron

This comment has been minimized.

Copy link
Member

commented Apr 20, 2017

So here's an idea for avoiding accidental double-spacing, although it doesn't quite work (see below), but i figured it was worth noting down anyway:

  • Instead of a line-height-step: <length> property that is inherited, have something like line-height-step: <number> | auto | none which is not inherited, but that controls a step size (which is conceptually either none or a length) that is propagated from ancestors to descendants:
    • the initial value is auto, which means the lines of this element use the same step size as the (containing block? / parent?), if any
    • the none value means the lines of this element do not use a step size
    • the <number> value means this element's step size (a length) is computed based on the element's properties. Values smaller than 1.0 are invalid (or changed to 1.0). The step size is computed by computing the height a line box would have if it contained glyphs from all fonts in the font-family list (given the other font properties), i.e., using the ascent and descent of each, and the line-height (which might be normal and thus lead to using external leading), and then multiplying that result by the <number> value.

(I'm not particularly happy with the names I've chosen here.)

So the problem with this as that it doesn't actually solve the font fallback problem, since font fallback often fails to find a glyph in the font-family and has to fall back to searching for other available fonts.

@dbaron

This comment has been minimized.

Copy link
Member

commented Apr 20, 2017

So on further consideration, maybe font fallback isn't as much of a problem as I thought.

In particular, line-height-step isn't actually aligning baselines to even spacing; it's aligning the line boxes. Font fallback can cause the position of the baseline to vary within the line box, but maybe that just leads to slight baseline jitter rather than accidental double spacing. Maybe that's not so bad. (Although it's not great that it would defer having a solution for real baseline snapping.)

Then I think the remaining question is that we don't actually know what line-height: normal means (#1254), and which fonts' metrics it considers. If all of the font metrics that could ever be considered for line-height: normal are considered when evaluating the step length from line-height-step: <number> as I propose above, then I think things would be ok.

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 20, 2017

I believe I understand how your proposal work. But I will need to think about it for a while to figure out if:

  1. solves the problem ( I think it does)
  2. solve the problem better than my thing (I don't know, that might depends on how line-height works)
  3. Whether it will make sense to authors and let them express the thing they want in an understandable way (I do not know yet).
@astearns

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

I agree that in your example the double-spacing of the Myanmar and large capital text in the second box is largely intentional, and not relevant to this issue.

The Arabic case might be relevant. I think that if there were browser differences in dealing with how the Arabic line height interacted with a line grid such that one browser double-spaced the lines and another did not, that's the case that this issue is worried about. But it looks to me like this example should be fairly clear-cut - the Arabic line-height is several pixels larger than the grid spacing. The third box doesn't look like an improvement to me, because it's tightening the specified line-height too much to squash the lines into the grid.

So I think the example would have to be a closer call for this issue to come into play. The line-height needs to be closer to (and smaller than) the grid spacing, such that there's clear authorial intent to not get double-spaced lines. This issue is then relevant if there are enough differences in text rendering between browsers and platforms that accidental double-spacing occurs.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2017

Thank you for the confirmation, that is really helpful.

I agree that in your example the double-spacing of the Myanmar and large capital text in the second box is largely intentional, and not relevant to this issue.

This is great, this is I was most worried about. Having your thoughts helped me a lot, thank you.

The Arabic case might be relevant. I think that if there were browser differences in dealing with how the Arabic line height interacted with a line grid such that one browser double-spaced the lines and another did not, that's the case that this issue is worried about.

Great again, thank you. Yeah, there will be differences by browsers/platforms, both in Line Grid and Rhythmic Sizing, because font metrics of the same font name varies by platform or even by versions of the same OS, and as we figured out, how browsers compute the height of line-height: normal vary.

What I don't understand is, how we could solve this when things are working exactly as intended by font designers but does not match to authors intention. I'm feeling we need to rely on heuristics, but I'm not confident of this part of my understanding yet.

But it looks to me like this example should be fairly clear-cut - the Arabic line-height is several pixels larger than the grid spacing. The third box doesn't look like an improvement to me, because it's tightening the specified line-height too much to squash the lines into the grid.

Great, so you think Arabic should do the 2nd box, do I understand correctly? I agree, and I was afraid people prefer the 3rd box. If a browser with the hard-coded minimum of line-height implements Line Grid, it will produce the 3rd box. Is this difference what we want to solve in this issue?

So I think the example would have to be a closer call for this issue to come into play. The line-height needs to be closer to (and smaller than) the grid spacing, such that there's clear authorial intent to not get double-spaced lines. This issue is then relevant if there are enough differences in text rendering between browsers and platforms that accidental double-spacing occurs.

I don't understand this paragraph...are you suggesting to change the example?

@astearns

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

I am suggesting the example needs changes. As it is, it does not seem relevant to this issue to me.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Sep 27, 2017

Google Translate helped me for the first sentence, then understood the rest. Did you write "smaller" when you want "slightly larger"? If the used line-height is smaller than grid, there will be no double spacing.

To adjust the used line-height in details, I need to look into font metrics value in the debugger, but I don't have WebKit build right now. I'll try to get one, but it looks like my understanding of the problem is correct. My next question is do people agree this is a heuristic problem, but let's discuss in the call.

@css-meeting-bot

This comment has been minimized.

Copy link
Member

commented Sep 27, 2017

The Working Group just discussed Avoiding accidental double spacing.

The full IRC log of that discussion <dael> Topic: Avoiding accidental double spacing
<tantek> is it just a button? I thought this was the first draft on the new system?
<dael> github: https://github.com//issues/938
<tantek> thanks astearns, noting here FTR: https://drafts.csswg.org/css-multicol/#changes
<koji> https://github.com//issues/938#issuecomment-330561882
<dael> koji: The biggest problem for me isI'm failing to understand definition. I made this example ^
<dael> koji: astearns gave me feedback that...the left most in the e xample is normal. Second I applied line-grid. I applied line-box contain to third.
<dael> koji: Second block has some double spacing but astearns says they're all intentional. Is that common understanding within this group?
<dael> myles: I think you're asking me?
<dael> florian: Everyone.
<dael> koji: Yeah. We need to distinguish intentional and accidental double spacing and there isn't a clear definition.
<dael> koji: The second box is [missed] it happens accidental when font metric is only slightly larger. is that correct?
<dael> astearns: one thing I missed when I commented is that in your example you are not chaning line height. it's all the same, but there are some fallback fonts making some lines taller.
<dael> koji: Correct. it started with line-height normal and it does double step.
<dael> astearns: Right. With that new understanding I believe you are correct that this is an example of the problem stated by the issue. 2nd is accidental line spacing because the author had line height normal and the grid set to something without double spacing.
<rachelandrew> someone will need to point me to how to publish the WD, me being a newbie.
<dael> astearns: IN my mind this is an intentional result of the feature. You use rhythm to get consistant spacing, but the content is such that you don't get it, so the spacing is forced by the grid so the result is what authors should expect.
<dael> koji: Okay.
<dael> koji: Are you saying this is intentional or acicdedntal?
<dael> astearns: Accidental in that if the author didn't know what content would go into their grid, they didn't have control over the fonts used, the person settin gup line grid might not have expected it. But they chose a rhythm, chose a grid, so we're fitting author intent of using a grid.
<dael> astearns: I think this is an example o f the issue as stated. I personally don't think the issue is terrifically important because author said they wanted to use a grid or rhythm.
<dael> koji: Thank you, that is exactly my understanding. florian do you have different opinion?
<dael> florian: I'm struggling with how to say my opinion in 10 minutes when in last F2F we tried to discuss and didn't conclude in 2 horus.
<dael> florian: I think when you have a case of line-height normal and then step to a value and large fallback the feature works as intended. Similar case, technically, is line-height: normal line-height-step to a specific value, and the main font on the engine happens to be a little too big and everything is double spaced. That' snot what authros want and I don't know how to fix that.
<dael> florian: First problem happens with line-grid but the second one you don't have the same problem becuase line-grid falls out of main font size.
<dael> koji: but if you go to single grid in that case lines overlap. Grid is fixed height, line-height is normal, and main font is lager.
<dael> florian: Line-grid you don't set it. That's the point.
<dael> koji: what you're saying is line unit is fixed size and line-height: normal and the primary font is taller then spec. unit. Is that the case?
<dael> florian: I think yes, I didn't hear you clearly.
<dael> koji: If you compress the font to a single unit they overlap. You said the font is taller then the unit, right?
<dael> florian: What unit?/
<dael> koji: Let me confirm. You said unit is fixed size.
<dael> florian: I said line-step-height is fixed, line-height is normal. That font on that system gives a line-height taller then step you set for primmary font.
<dael> koji: If the font is taller and you try and fit in a single step you overlap, right?
<dael> florian: Line-height is normal. They're larger then your step so you double space everything.
<dael> koji: Yousaid that's a problem?
<dael> florian: Yes.
<dael> florian: Double space everything is a problem if the rhythm works in general and some fallback is taller that's working as intended. If everything is double spaced that's not working as intended.
<dael> florian: Primary font...I can't do this in 5 minutes.
<fantasai> florian: I tried for hours and failed.
<dael> koji: [missed] We mostly discussed which features people break and htat's not related.
<dael> Rossen_: For the sake of furthering this, I think in summary I've heard that florian's main objection is in case of fallback font being slightly lrge then primary you will have double spacing when fallback font is used.
<dael> florian: That is how it works and ingeneral not a problem.
<dael> Rossen_: And because of this you expect everything will have double spacing?
<dael> florian: No, what I'm saying is when the autho sets a value for line-step-height they don't know what the result of line height calculation will be so they cannot set it in a reliable way. So there is a possibility that things will look right on one browser and not in the other because different font metrics. That's not what hte author wants and a limmitation. The double space everywhere on every brwoser is what's wanted.
<dael> koji: I think I understand your point much better. I'll prepare another test to see if my understanding is correct.
<dael> Rossen_: Sounds great. How about we try and wrap here. Seems like there's more clarity for koji in test cases you need to look at. Why don't you create the test cases and bring them back.
<dael> Rossen_: We'll take time during F2F on this, but if we can resolve before even better.
<dael> Rossen_: Okay for both of you?
<dael> koji: Okay for me.
<dael> florian: I have no obj to koji trying things out. I don't think it'll change what I think of this feature.
<dael> Rossen_: That's the top of the hour. Have a great day/night and we'll talk next week.
<tantek> next week is different time right?
@astearns

This comment has been minimized.

Copy link
Member

commented Sep 27, 2017

I agree with the point Florian made in the minutes above. The problematic case isn't one where there are some fallback fonts that cause things to be double-spaced. The real problem is if there's a fallback that causes everything to be double-spaced.

The simplest example I can think of is one where you have two fonts that the author specifies - FontA and FontB. FontA is present on the developer's machine, but they know it isn't present on some other platform so they list FontB as a backup. All of their content can be rendered in FontA or FontB. Unfortunately the developer does not know that FontB has metrics that make the default line-height slightly larger.

As currently defined, with line-height-step the author has to choose a value, and they will choose one that works with FontA. If the value they choose is too small, all of their content will be double-spaced on the FontB platform.

As currently defined, with line-grid the author does not choose a value. By default the line grid will be established by the font metrics of FontA on the developer's machine, and by the font metrics of FontB on its platform (since it's the first available font). So content will not be double-spaced with line-grid in this case.

Is this a fair restatement of the issue, @frivoal?

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 29, 2017

Is this a fair restatement of the issue?

Yes, this is excatly what I meant.

In addition, I'll also note that similar things can happen even if the author only specifies "FontA", in several different scenario:

  • Similarly to what you said above, FontA is available on one system but not the other. On the other, instead of FontB, you get some default system font. Otherwise the logic is the same.

  • "FontA" is a webfont defined through an @font-face, and there's some network connectivity issue of some sort, and FontA doesn't get downloaded, so you get some default system font instead, and get the double spacing vs not double spacing problem without even having to change browser.

  • The font metrics of "FontA" are computed differently by different browsers. For an example, see this font I was using in line-height tests: https://github.com/frivoal/web-platform-tests/blob/line-height-experiments/css/css-line-height/support/Revalia.woff Chrome gives this font a significantly taller line-height than firefox does. So for this font, if an author developped the page on firefox and picked a size that worked fine there, you could get double spacing everywhere in Chrome despite using the same font.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 2, 2017

Thank you for your replies, it gave me a new understanding of the problem.

Is this issue suggesting to add e.g., auto to compute the step unit from the used line height? If that's the case, I'm ok with it, it's just not in any use cases and no one made such requests ever.

With Line Grid, as far as I talk to web developers, they normally want different line height from grid, so their usual CSS looks like:

.line-grid {
  line-height: 1.4;
  -webkit-line-grid: create;
  -webkit-line-snap: contain;
}
.line-grid > * {
  line-height: 1.2 or normal or some other values;
}

If author uses normal to make the design i18n-friendly, here is the modified example jsbin (again for Safari to view). As you can see it, if the content uses taller fonts, there's a fallback that causes everything to be double-spaced.

2017-10-03 4 07 35

So I don't see much differences, but I understand our assumption on how authors use Line Grid is different, and if auto can save some English cases, I'm fine to add it.

@astearns astearns removed the Agenda+ label Oct 3, 2017

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 4, 2017

Is this issue suggesting to add e.g., auto to compute the step unit from the used line height? If that's the case, I'm ok with it.

I don't think that is what was being suggested, but I suppose it would make things better.

As I said in #938 (comment):

  • When setting the line-heigh-step, you (almost?) never want to set it to something smaller than the line height of the element that establishes the rhythm.
  • When inheriting line-height-step, you do want it to be able to be smaller than the line height of the element it is inheriting into. That is what enables things like titles to be enlarged to a multiple of the step size.

This means that if we go with an auto value, we probably needs to reuse a logic similar to the one I suggested in #938 (comment), where auto computes to the height of the line in px. That way, when you set something to auto, it gets the right height, but it keeps that same height when inheriting on descendants, preserving the rhythm.

So that would give us something like this:
line-height-step: none | auto | <length>:

  • none computes to (or behaves as, I don't care) 0
  • <length> does the same as what the spec says now
  • auto computes the height in px the line would have if it only contained the strut.

And this should come with a large warning telling authors to use auto rather than <length> to avoid interop problems.

Or maybe a slightly more expressive vairiant (based on what I suggested back then):
line-height-step: none | auto | [ <length> [ safe | unsafe ]? ]

  • none computes to (or behaves as, I don't care) 0 unsafe
  • auto computes the height the line would have if it only contained the strut, in px, unsafe
  • if both safe and unsafe are omitted, it is the same as safe
  • <length> unsafe does the same as what the spec says now
  • <length> safe computes to max( auto, <length>)

I like that a better because:

  1. It is a little more expressive
  2. it forces authors to type "unsafe" to get the behavior that has interop risks, and hopefully this will have enough of a deterrent effect that things will be alright.

So I don't see much differences, but I understand our assumption on how authors use Line Grid is different, and if auto can save some English cases, I'm fine to add it.

I do not think this has anything to do with English vs Japanese. The example @astearns gave in #938 (comment) and the ones I gave in #938 (comment) are equaly likely to happen in any language.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2017

Great, we're getting to the consensus.

When setting the line-heigh-step, you (almost?) never want to set it to something smaller than the line height of the element that establishes the rhythm.

People wants half-rhythm. This doesn't align lines across columns, but still gives a good rhythm, and consumes less spaces, and that this is suitable for mobile scenario. For this use case, safe/unsafe doesn't express its intention very well.

none was discussed before, but fantasai didn't like having two ways to turn it off, and we can't eliminate accepting 0, so it was removed.

So, shall we start with auto | <length> and see what it happens?

Also are you ok to add <length> to Line Grid as well? This is the largest feedback I've got so far in terms of numbers.

@astearns

This comment has been minimized.

Copy link
Member

commented Oct 4, 2017

If you're getting feedback that line grid needs a <length> somewhere, please open a separate issue so that we can discuss the use cases. I haven't been convinced it's necessary.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2017

please open a separate issue so that we can discuss the use cases. I haven't been convinced it's necessary.

Ok, thanks, will do so.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2017

Alan's suggestion above gave me a hint for another idea.

What do you think about

line-height-step: none | auto

?

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2017

agenda+

Proposals

  1. line-height-step: none | auto | [ <length> [ safe | unsafe ]? ]
  2. line-height-step: match-parent | none | auto

Background

As I understood last week, @frivoal is not trying to solve the problem itself but wants to solve by making it harder for authors. I, and IIUC @tabatkins / @shans / @dbaron as well, were all trying to solve, with heuristics or with side effects, so none sounded to him. I didn't understand his proposal because it didn't seem to solve, until I understand he's trying to solve by making harder.

Proposal 1 makes this specific case harder to appear unless author adds unsafe keyword.

Proposal 2 does so unless author adds line-height property twice, once to set to a fixed value and then to set to normal.

I prefer proposal 2, for the simplicity for authors. Also it makes compatible with Line Grid that we can discuss if/how to support <length> for both features together.

EDIT: originally, none|auto but it doesn't work, so match-parent was added.

@kojiishi kojiishi added the Agenda+ label Oct 8, 2017

@astearns

This comment has been minimized.

Copy link
Member

commented Oct 8, 2017

The worry I have with going with none | auto to start, then introducing <length> later is that we still run into all the problems described above when you re-introduce <length>. Since auto is not the default it just becomes one of the options, and when the author opts into a <length> the differences between this length and the specified and used line-height all come back into play.

@astearns

This comment has been minimized.

Copy link
Member

commented Oct 8, 2017

@koji, please do not characterize another working group member's efforts as "not trying to solve the problem." Let's continue the discussion here and come to a better understanding of both @frivoal's proposal and @dbaron's new writeup https://github.com/dbaron/css-line-spacing/blob/master/explainer.md before we come back to this on the call.

@astearns astearns removed the Agenda+ label Oct 8, 2017

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2017

@astearns sorry for my vocabulary, I can't find good words to express "we would like the problem to persist if author added a unsafe keyword." That's what I understood, but great if I were wrong.

Also note that given Mozilla's feedback in Paris, Japanese companies started creating sites. I hope they can use new syntax if we were changing.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2017

I guess I understood what you mean better now, better way to say it is "solve it by making it harder for authors". Sorry about that, I'll edit.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2017

For your first comment, I'm thinking we can live without <length>. Hopefully that resolves your concern?

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

One more thing; @fantasai tried to solve this issue 6 years ago for Line Grid, but we chose not to solve it. Maybe we can learn from her? (disclaimer; sorry, no offence, "not to solve" is a good word in my culture but it looks like I'm wrong, haven't learned better way to express this.)

@frivoal

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 10, 2017

@kojiishi, my proposal had length and unsafe and safe and all that stuff because as far as I could tell, you insisted on having length, and I was trying to make that safer without removing length. As you know say you can live without it, things get much simpler, and we don't need all that stuff anymore.

So, between the current spec and

Proposals

  1. line-height-step: none | auto | [ [ safe | unsafe ]? ]
  2. line-height-step: match-parent | none | auto

I prefer proposal 2, without much hesitation.

On the other hand, I will need a bit more time to think carefully between this vs dbaron's proposal, vs current line-grid, to understand if they are complementary proposals, or if one makes the other unneeded, and if so, which one.

@fantasai

This comment has been minimized.

Copy link
Collaborator

commented Oct 11, 2017

@kojiishi @frivoal While this does seem to be an improvement in addressing some of the concerns, I think it is improper for us to move forward with this feature without any sort of concrete, realistic use case to justify it and to ensure that our design is fullfilling it. I believe @kojiishi had an outstanding action item to provide some, and I think I'd prefer to have those use cases to discuss first so that we actually understand what we're trying to solve before we decide how we want to solve it.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

@fantasai can you open a new issue, or request discussion by e-mail? It's a great feedback, but not related with this issue.

It's a great feedback that it means the Tokyo session was a failure -- it was one of the biggest objectives to get understanding. I think this is hard because both sides discuss based on their common sense, but the common sense is different, and neither side understand how. I'm getting better understanding how Latin vertical rhythm is different, so I think I can do that better next time.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

Another idea from a Japanese feedback is similar to Shane's heuristic idea, but apply the heuristics to the rhythm unit rather than line height. This makes sense to me, because as it was pointed out in earlier comments in this issue, this happens when the rhythm unit is between 1.0 and 1.2. When above 1.2 and if double spacing occurs, it's very likely to be intentional than accidental.

I'll try to summarize proposals so far.

@kojiishi

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

Summary of proposals so far:

  1. Make it harder for authors
    1.1. line-height-step: none | auto | [ [ safe | unsafe ]? ] (@frivoal)
    1.2. line-height-step: match-parent | none | auto
  2. Heuristic
    2.1. Adjust slightly larger normal height to the unit (@shans @tabatkins, @fantasai's effort looks similar to this)
    2.2. Adjust rhythm unit between 1.0 and 1.2 to 1.2
  3. Non-heuristic
    3.1. Item 4 of Better spacing of lines in CSS, an explainer (@dbaron)
    3.2. line-box-contain: block glyphs, similar to 3.1 but opt-in (Hyatt, 6 years ago.)

Category 1 is not needed for Line Grid, but category 2/3 are probably needed for Line Grid as well.

EDIT: Note, in the comment above, @frivoal said the original intention of this issue is about making it harder, but it looks like Alan and dbaron's proposal extended the coverage of this issue, so this summary includes both. I'm open to discuss these two together, or separately.

@kojiishi kojiishi added the Agenda+ F2F label Nov 1, 2017

@astearns astearns removed the Agenda+ F2F label Nov 6, 2017

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.