w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.49k stars 660 forks source link

[css-text] Having overflow-wrap: break-word affect min-content size is not webcompatible #2951

Closed upsuper closed 6 years ago

upsuper commented 6 years ago

We resolved in #2682 that overflow-wrap: break-word should affect min-content size, however, it doesn't seem to be web-compatible.

After we landed the change to Gecko, we immediately got two bugs reported, and the latter is an issue on GitHub, see https://github.com/vscode-icons/vscode-icons/wiki/ListOfFiles.

GitHub has word-wrap: break-word set on the whole page, likely because its content are from users, and it's better to allow breaking word as a fallback when there is an otherwise unbreakable string. However, in that case, people may not really want it to affect min-content in e.g. tables accidentally.

CShepartd commented 6 years ago

First is for me wrong usage CSS. Second is more accurate, but like was discovered in comment almost the same thing showing on Chromium when you put word-break: break-word on the table.

upsuper commented 6 years ago

the same thing showing on Chromium when you put word-break: break-word on the table.

That basically indicates there are use cases for either way, word breaking with intrinsic size affected, and without. And for GitHub's case, I think it is a reasonable example of the latter case.

fantasai commented 6 years ago

I don't think it indicates use cases either way, it indicates that people are unintentionally relying on it not having any effect in certain shrinkwrap cases (and being upset that it doesn't in others). Which is a case for being more specific about where it's applied. And I agree with idontlikespying’s comment in 1476180, the CSS there is nonsensical.

If tables are a particular problem, adding table { overflow-wrap: normal; } to the UA stylesheet could be considered. Then the author can adjust their approach if necessary. (Given that data frequently gets stacked side-by-side and is especially likely to get unexpectedly squashed, this is a pretty reasonable style rule to add.)

CShepartd commented 6 years ago

@fantasai I agree. This solution sounds good or just not inherit that rule for tables by default from higher levels.

upsuper commented 6 years ago

There is another bug filed for PHP manual, where putting table { overflow-wrap: normal; } wouldn't help, because what they do is code { word-wrap: break-word; } and then using <code> in every table cell of one column.

CShepartd commented 6 years ago

@upsuper I dont see any bug. They should set min-width for td or code. Look at this table on lower resolution in stable FF. Table is breaking layout. In Nightly it calculating proper and not break layout. This is fix, not a bug. Attachment with example

fffixtable

morrisonlevi commented 6 years ago

php.net will definitely need to revisit its layout.

However, none of the columns have predefined widths; that's purely auto-layout. And yet, it's breaking letters of the word in that first column before trimming down the last? Seems wrong.

Edit: Any specific advice on php.net is appreciated. Setting a predetermined width seems problematic, because we have many tables all throughout the site with various content per-column, with various numbers of columns, and these are rendered automatically from DocBook sources. Maybe table-layout: fixed? That has its own problems, notably further down on the same page where we have a two-column table where the first column is very skinny.

fantasai commented 6 years ago

@morrisonlevi The suggestion would be table code { word-break: normal; }, which resets word-break to normal so that breaks within words are disallowed (if that was the intent, which it seems it was).

FremyCompany commented 6 years ago

Based on the amount of reported issues, maybe we should revisit not accepting the word-break: break-word property value. Sure, that value doesn't make a lot of sense, but also it is now used a lot on the web, and we might just as well cave in.

We have seen a couple of websites doing word-break: break-all; word-break: break-word which are somewhat broken in Edge with the new text layout engine because we now properly support break-all on inlines, so I guess those sites were already broken in Firefox in the past. The only way to fix them would be to support break-word so we might end up doing just that.

morrisonlevi commented 6 years ago

@fantasai PHP.net is okay with breaking words, which is sometimes necessary in other places. It's just in this case it is odd (at least to me) that it was chosen to break the words instead of shrink the last column. I believe _ is not considered to be a character that would break a word, correct? Maybe we need to be adding <wbr/> tags around the underscores when rendering them.

The new behavior seems better generally, just not in this case.

css-meeting-bot commented 6 years ago

The Working Group just discussed Having overflow-wrap: break-word affect min-content size is not webcompatible, and agreed to the following:

The full IRC log of that discussion <dael> Topic: Having overflow-wrap: break-word affect min-content size is not webcompatible
<dael> github: https://github.com/w3c/csswg-drafts/issues/2951
<dael> xidorn: I put in change to make overflow-wrap:break-word to effect min content. We immediately got 4 bugs. fantasai suggested we can make table reset the overflow-wrap to normal which fixes 2 of them.
<dael> xidorn: I want to bring it to WG and see what we do next
<dael> florian: What are remaining 2? PHP manual thing was one?
<dael> florian: It looks like a strangely coded web page.
<dael> florian: What's last?
<dael> xidorn: I think fantasai and the [missed] said it was wrong use and we can ignore
<dael> dbaron: We're talking about 4 bugs and it's on the nightly channel
<dael> fantasai: I'm not sure what to do. I think alternatives are terrible and web compat is a problem
<fantasai> s/and/but/
<dael> myles: We don't have a choice. 4 bugs on a nightly. Webcompat is more important
<dael> florian: I suggest we try the UA stylesheet change and see. If the change on tables solves the general problem maybe we're back. If not we have to do something else
<dael> frremy: My PoV I don't believe a quick change will fix. At MS we have other issues we're seeing where we'd like to reconsider word-break:break-word where we had websites change to break-all and Edge breaks in the middle. I know we supported not supporting it, but I'm becoming less convinced we can't. So supporting break-word is an option on the table. We should consider that as a part of this
<dael> fantasai: WE have someone from PHP engaging on issue. Example where table is not breaking, behavior they want ideally is that you have 2 types of min-content? They want table to wrap at normal word breaks but not in overflow wrap cases unless it's necessary. If you look at example there is text that could wrap more without breaking code words and they expect those breaks take priority.
<dael> fantasai: Not sure where to go with that, but it occurred to me that we're not handling that type of prioritization correctly in general
<dael> florian: So I felt we did find 4 bugs and if we fix UA we fix 2 and the other bugs were different
<dael> dbaron: Does the fix break what the sites were intending?
<dael> fantasai: Sites broken intended to not apply inside the table.
<dael> dbaron: I think you need to be careful on intent. A lot of sites might have waned current behavior where it allows long content to break
<dael> florian: Tables doesn't do that
<dael> dbaron: Okay
<dael> dbaron: b/c changes min-content
<dael> florian: Right, so table won't shrink to where it could break
<dael> fantasai: [sigh]
<dael> florian: I'd like Moz to try the UA stylesheet fix and see where that gets us. Maybe not enough, but good to check.
<dael> florian: If that doesn't break the web more it proides a clean path for new authors and maybe on top of that we still need word-break:break-word for frremy's reasons
<dael> fantasai: Idea case maybe is that...One case was someone put content in a 0 width continer and said I'd like you to word-break:break-word but shrink wrap took effect so it didn't. Other cases it seems there's multi levels of prioritization where we have 2 and authors would like 3. That's a big thing to think about.
<dael> fantasai: Overflow-wrap breaks are deprioritized in terms of when we accept the break. We're not able to deprioritize in terms of min-content width.
<dael> fantasai: For word-break:break-word it's awful ergonomics for CSS to add it. Authors are already confused on breaking prop and what called
<dael> florian: That's why I think we should keep trying and maybe spec as a corner
<dael> fantasai: If we have toe revert this I'd prefer a keyword to overflow-wrap that's the same as break-word but also effects min-content like 'overflow-wrap-anywhere' that's the same as word-break:break-word but lets author consider like multiple properties. word-break:break-word and word-wrap:break-word can't be combined
<dael> myles: Have we veered from original topic?
<dael> fantasai: Going back to this one
<dael> fantasai: I think it would be interesting to see if adding UA stylesheet rule would make it tolerably compat, but I'm not an impl so I'm not qualified to have an opinion for how that would be as an experiment.
<dael> astearns: Given we don't have clear solution I think trying this in browser stylesheet and getting more data is reasonable next step
<dael> florian: Might not be enough but good start
<dael> astearns: xidorn and dbaron sound good to try?
<dael> xidorn: I think we can try that. Easy enough to add it
<dael> astearns: Objections to Try a new rule in UA stylesheet and see how it impacts rate of bugs for FF?
<dael> RESOLVED: Try a new rule in UA stylesheet and see how it impacts rate of bugs for FF then come back to this
fantasai commented 6 years ago

@morrisonlevi I see what you're saying. It totally makes sense. The problem we have is that while line breaking has two levels of priority, table column sizing (and sizing in CSS in general) is only able to account for one. I think it would make sense to have that additional level of priority... but it would be a pretty major change in the CSS layout algorithms so it would be really difficult to do! I do wish we could do it, so that your table example would work as you want it to ideally; but I"m not sure that, practically-speaking, we can.

CShepartd commented 6 years ago

@morrisonlevi Like i wrote few day ago, just set min-width for code or that specific td. Then you will not need any <wbr/> which is for me bad practice for dynamic content

upsuper commented 6 years ago

FWIW, another bug on YouTube: webcompat/web-bugs#18202

upsuper commented 6 years ago

The YouTube case is probably very similar to the first bug that it's also a case where an absolutely-positioned element gets pushed against an edge of its container.

upsuper commented 6 years ago

webcompat/web-bugs#18169 this one is related to table, because the page apply word-wrap: break-word on td directly, so it's not fixed by the table trick.

upsuper commented 6 years ago

If other browsers don't want to follow Firefox to change the behavior, we would likely revert our behavior.

So far there are still four webcompat issues unresolved. Except the php.net one which can be considered to be buggy either way, others are real regressions (even if you think it's wrong use of CSS... but we know authors do lots of crazy things already). Specifically, our release manager seems to think www.ancestry.com in the first bug is a major site, and consider the regression to be potentially widespread.

I also talked with @karlcow in our webcompat team and he doesn't think having overflow-wrap: break-word affect intrinsic size would even fully compensate the lack of word-break: break-word support, as authors nowadays may use word-break: break-all as the fallback of word-break: break-word.

If Blink can make progress on their bug maybe we can try keeping the new behavior for longer and see, otherwise we would probably back it out.

kojiishi commented 6 years ago

We are fine with whatever resolutions though we don't have timeline yet. fantasai and Florian said this is multi-decades effort to let authors to upgrade their sites to the new behavior.

Did Gecko turn to be positive about word-break: break-word? IIUC we're doing this effort because Gecko is negative. If still negative, and if we consider that we won't take this route, we will need to come up with yet another proposal.

karlcow commented 6 years ago

to add on what @upsuper said here and because of the initial discussion in #2390

This is a pattern we see on the Web which is partly creating this issue.

Example https://github.com/webcompat/web-bugs/issues/18205

.brz .brz-rich-text {
    overflow-wrap: break-word;
    word-wrap: break-word;
    word-break: break-all;
    word-break: break-word;
}

One of the issues we see, even with the new version of overflow-wrap: break-word;.

Which ever order in the cascade, for browsers not implementing word-break: break-word; (ie, edge, firefox), the word-break: break-all; will take precedence over overflow-wrap: break-word;.

upsuper commented 6 years ago

Did Gecko turn to be positive about word-break: break-word? IIUC we're doing this effort because Gecko is negative. If still negative, and if we consider that we won't take this route, we will need to come up with yet another proposal.

So, I think the motivation is that the line breaking is already complicated, and we (as the working group) don't want to add another complexity onto that.

I don't think this really changes our attitude about word-break: break-word. It would be good if we can follow this route, but we (Gecko) probably cannot bear the cost by ourselves, so we would appreciate if other browsers can try doing the same change as well, preferably within a similar time frame.

I suppose that the behavior change should be reasonably easy to implement in Blink and WebKit as you already have all the code needed (for word-break: break-word), and there have already been some web-platform tests added as part of Gecko's implementation (in web-platform-tests/wpt#12002).

kojiishi commented 6 years ago

I can't commit on the timeline at this moment, sorry, given, as you said, this would require quite a time. How about other engines?

CShepartd commented 6 years ago

Do you know why we now have problems with word-break: break-word/overflow-wrap: break-word? Cuz W3C can not decide to solution fxing a problem. On the one day they are supporting word-break: break-word and on the next day they changing their mind and revert word-break: break-word. On the one day they are adding calculating min-content size in overflow-wrap: break-word, on the other day they are revert this. Is this a description for a standard or a mess? How a web-devs or even browser-devs can take changes for real? We are in this point because of indecisiveness. Not webcompatible? Maybe we need to fix it once without compomises? (not webcompatible was also because of lack this feature).

frivoal commented 6 years ago

On the one day they are supporting word-break: break-word and on the next day

There was never a group resolution to add that feature under this syntax. It made it to the drafts by mistake, and was removed as soon as the mistake was noticed. There has always been strong reluctance to adding it, not because it is not useful, but because it makes everything else more complicated.

On the one day they are adding calculating min-content size in overflow-wrap: break-word, on the other day they are revert this.

This has not been reverted. This was added to the draft with the hope that this addition was possible, and we're in the process of checking whether it is. There's a chance that it is not. If so, that will be reverted.

Is this a description for a standard or a mess? How a web-devs or even browser-devs can take changes for real?

The current situation on the web is a mess. The world often is. We are attempting to find a non-messy solution to a messy problem. Maybe compatibility constraints will prove this impossible, and we will need a messy solution (word-break: break-word is considered messy). Maybe we will solve it cleanly.

Here is what the "Status of the document" says:

This is a public copy of the editors’ draft. It is provided for discussion only and may change at any moment. Its publication here does not imply endorsement of its contents by W3C. Don’t cite this document other than as work in progress.

or, if you are reading this on TR/

Publication as a Working Draft does not imply endorsement by the W3C Membership. This is a draft document and may be updated, replaced or obsoleted by other documents at any time. It is inappropriate to cite this document as other than work in progress.

Not webcompatible? Maybe we need to fix it once without compomises?

If there was a solution that did not involve any compromise, we would have taken it.

Taking work-break: break-word as upsides (adds the feature, fixes existing sites), and down-sides (it makes everything harder to understand, maintain, and evolve). Make overflow-wrap: break-word affect sizing has upsides (adds the feature, it fixes some existing sites, fits the system better), and downsides (doesn't fix all sites, breaks some). Adding a new value to overflow-wrap that behaves like break-word but also affects sizing has upsides (it doesn't break anything, adds the feature, fits well into the system), and downsides (sites need to be updated to opt into the fix, two values for almost the same thing seem unfortunate).

So we're weighing the pros and cons, and to figure out the full extend of the compatibility problem, the most efficient way is to try to implement in test builds and see what happens.

Nobody is happy about this situation. But there isn't an obviously right answer we can all just get behind, so it might take a while to solve.

upsuper commented 6 years ago

Another case webcompat/web-bugs#18248 is filed.

This case is an interesting one which has different pattern than others we've seen so far. It is caused by nested absolutely-positioned element without any explicit width specified. A simplified version can be found here: https://jsfiddle.net/0qng2eaf/

upsuper commented 6 years ago

Facebook joins the list: webcompat/web-bugs#18113.

It seems this case it's flexbox, which is exactly the case that we want to fix with making overflow-wrap: break-word affect intrinsic size, and Facebook relies on it doesn't do so.

I'm going to put the new behavior on Gecko behind a pref and turn it off by default in bug 1484587. We can see what to do after that.

karlcow commented 6 years ago

Reduced test case for Facebook case in https://github.com/webcompat/web-bugs/issues/18113 https://codepen.io/webcompat/pen/KxPJPa

fantasai commented 6 years ago

@upsuper Thanks for the update, sounds like we'll need to revert the change. Agenda+ to approve that. We'll still need to solve the use case, so gonna re-open #2682 for that.

css-meeting-bot commented 6 years ago

The CSS Working Group just discussed Reverting resolution about overflow wrap break word.

The full IRC log of that discussion <gregwhitworth> Topic: Reverting resolution about overflow wrap break word
<astearns> github: https://github.com/w3c/csswg-drafts/issues/2951#issuecomment-421705003
<gregwhitworth> florian: we tried to have break-word affect intrinsic sizing
<gregwhitworth> florian: we had some hope that changes int he UA stylesheet would be sufficient, but it is not true unfortunately
<gregwhitworth> florian: so the only option it seems is to revert the previous resolution where it does not affect intrinsic sizing
<gregwhitworth> astearns: is that seperate issue somewhere?
<gregwhitworth> fantasai: yeah we re-opened the issue
<gregwhitworth> astearns: objections?
<gregwhitworth> Resolved: Revert the previous resolution