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

Support "overflow-wrap: anywhere" #1520

Merged
merged 1 commit into from Dec 13, 2021
Merged

Conversation

aschmitz
Copy link
Contributor

This supports the overflow-wrap value anywhere: anywhere is like break-word, but the soft breaks it allows are considered when calculating min-content intrinsic sizes.

This can help alleviate the problems identified in #1153 (comment), but obviously doesn't close that issue.

I'm a little unsure on the test: if you have preferred ways to test this behavior, feel free to remove these tests. (Additionally, the break-word and normal tests would seem as though they should be 120px wide, not 100, but I can't figure out why they have their current values: I'd be interested in thoughts there.

This supports the `overflow-wrap` value `anywhere`: `anywhere` is like
`break-word`, but the soft breaks it allows *are* considered when
calculating min-content intrinsic sizes.
@liZe
Copy link
Member

liZe commented Dec 13, 2021

Hello @aschmitz,

Thanks a lot for this pull request, it’s just perfect 💜.

Additionally, the break-word and normal tests would seem as though they should be 120px wide, not 100, but I can't figure out why they have their current values: I'd be interested in thoughts there.

That’s a good question. We’ll check that and find why we get 100.

@liZe liZe merged commit 3c382e3 into Kozea:master Dec 13, 2021
@liZe
Copy link
Member

liZe commented Dec 13, 2021

The problem is caused by the trailing space whose size is removed from the minimum width. Looks like it’s removed twice :/.

I’ll remove the trailing space from the test, and add another test to check what’s wrong with it.

@liZe
Copy link
Member

liZe commented Dec 13, 2021

Moreover, the first test doesn’t pass with Pango 1.50.x. We already had to report a bug with tabs, let’s hope that this bug is also easy to reproduce and to fix…

@liZe
Copy link
Member

liZe commented Dec 13, 2021

We already had to report a bug with tabs, let’s hope that this bug is also easy to reproduce and to fix…

That’s actually a bug in Pango 1.48.x, fixed in 1.50.x. The problem is caused by hyphens, automatically added when words are broken. With Pango 1.48.x, when the aaaaa string was rendered with no width available, we got:

a
a-
a-
a-
a

Notice the missing hyphen on the first line.

As, in WeasyPrint, we render lines one by one, the hyphen was always missing. That’s why we got a width of 1 character.

Now, in Pango 1.50.x, we get:

a-
a-
a-
a-
a

That’s better! But the minimum width is then 2 characters.

The solution is to simply disable hyphenation when we break words.

liZe added a commit that referenced this pull request Dec 13, 2021
liZe added a commit that referenced this pull request Dec 13, 2021
@grewn0uille grewn0uille added this to the 54.0 milestone Dec 13, 2021
liZe added a commit that referenced this pull request Dec 13, 2021
When we calculate the minimum width of an inline block, the size of the
trailing space is already removed by split_first_line. There’s no need to
remove it twice.

We should probably fix split_first_line to remove the trailing space only when
it’s been asked to. But there’s no obvious situation when we want the minimum
width to include trailing spaces, as the minimum size requires line breaks
everywhere, including after each space.

At least, this commit doesn’t remove trailing spaces twice.

Related to #1520.
@liZe
Copy link
Member

liZe commented Dec 13, 2021

The trailing space bug is now fixed.

As far as I can tell, break-word actually works too, doesn’t it?

@liZe liZe added the feature New feature that should be supported label Dec 13, 2021
@aschmitz
Copy link
Contributor Author

aschmitz commented Dec 14, 2021

break-word isn't really supported yet, although in many cases overflow-wrap could be used instead.

There are two cases where this is a concern, and I mostly have to handle in advance without knowing which one I'm going to be hitting:

  1. Sometimes, there's a very long string that doesn't wrap naturally in a table cell (think a URL or enterprise Java class name). Right now, this requires overflow-wrap: anywhere because even with overflow-wrap: break-word, that string will count towards the table's minimum width, which overrides even an explicitly set maximum width for the table(!).
  2. Other times, I have a nominally fairly narrow column and a column with a lot of text. (Think "product descriptions and prices".) If I apply overflow-wrap: anywhere, the column width algorithm will calculate that the minimum width of each column is about one letter wide, and feed the remaining width of the page to columns in proportion to their maximum widths. That's very long for the product description, but very short for the price. As a result, the price column gets squeezed and all of the prices are forced to wrap, even though they wouldn't without overflow-wrap: anywhere.

Unfortunately I have no real way to know which is which upfront, since my users provide arbitrary content. I could set a minimum width for columns across the board, but that feels like a bit of a hack as well.

However, I can detect a long string that wouldn't otherwise wrap, and automatically enclose it with something like <span style="word-break: break-all">abcdefghijklmnopqrstuvwxyz</span>. That would let the layout engine wrap the long words anywhere it needs to, and still respect shorter things.

(Of course the "real" solution is having people specify column widths if necessary, which we also support, but it would be very nice to not require everyone to do that in common cases.)

@liZe
Copy link
Member

liZe commented Dec 14, 2021

break-word isn't really supported yet, although in many cases overflow-wrap could be used instead.

There are two cases where this is a concern, and I mostly have to handle in advance without knowing which one I'm going to be hitting: […]

I agree, overflow-wrap: break-word isn’t really useful for these use cases, and that’s much better with overflow-wrap: anywhere.

But, by "overflow-wrap: break-word actually works", I mean "overflow-wrap: break-word behaves accordingly to the specification". I think that it works correctly, now that Pango hyphens have been removed.

However, I can detect a long string that wouldn't otherwise wrap, and automatically enclose it with something like <span style="word-break: break-all">abcdefghijklmnopqrstuvwxyz</span>. That would let the layout engine wrap the long words anywhere it needs to, and still respect shorter things.

That would be the easiest way to handle long strings. Hyphenation is a nice solution for real words, but it’s quite useless for URLs for example.

@aschmitz
Copy link
Contributor Author

But, by "overflow-wrap: break-word actually works", I mean "overflow-wrap: break-word behaves accordingly to the specification".

Yep, you're right. I was misinterpreting that as word-break the CSS property, not break-word the overflow-wrap value. break-word should be working correctly now as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants