1253840 - text-align: justify; doesn't work together with white-space: pre-wrap;

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[mezood]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

I just have a text-align: justify; together with white-space: pre-wrap; .


Actual results:

The text didn't justify.


Expected results:

The text should be justified.

Comment 1

[Robert Utasi [:hunboy]]

This issue is fixed in chromium in 2014, so this is now a cross-browser compatibility issue.

For more details see: https://bugs.chromium.org/p/chromium/issues/detail?id=336636

Comment 2

[Robert Utasi [:hunboy]]

Attachment: testcase

Comment 3

[jscher2000]

This is an issue in the default WordPress editor (Gutenberg "blocks" editor), which uses white-space: pre-wrap; in the standard view.

• https://support.mozilla.org/questions/1303902
• Reduced test case: https://jsfiddle.net/f2bLq95s/

Applying or removing Justify alignment is not visible in the editor, you need to use Preview to see what you've done.

WordPress is a popular platform, but text-align: justify needs to be added using a plugin, it is not a native feature.

I'm not sure what priority that implies (priority is not set for this bug).

Comment 4

[shaulgo]

Is there any priority on this bug?

Comment 5

[Jonathan Kew [:jfkthame]]

I think it would be nice to handle this, though there are a few issues to keep in mind:

(a) The CSS2.1 spec explicitly said that browsers must not justify when pre-wrap is in effect:

If an element has a computed value for 'white-space' of 'pre' or 'pre-wrap', then neither the glyphs of that element's text content nor its white space may be altered for the purpose of justification.

So according to CSS2.1, the Firefox behavior here was correct. This is also reflected in web-platform-tests that Firefox and Safari currently PASS, while Chrome FAILS.

(b) The CSS Text 3 spec relaxed this requirement, leaving it up to the user agent to choose whether or not to justify in this case:

If an element’s white space is not collapsible, then the UA is not required to adjust its text for the purpose of justification and may instead treat the text as having no justification opportunities.

And the web-platform-test mentioned above has been annotated with MAY, to reflect that the behavior is no longer a requirement but an option.

So to change the behavior here would carry some risk of disrupting content that expects the CSS2.1-specified behavior. However, given that Chrome has been doing this for several years at this point, the compatibility risk is probably minimal. Still, sites (or tools) really shouldn't be relying on a behavior that is explicitly called out as optional in the spec, and seems to be implemented by only one of the major engines at this point. (Is there a corresponding webkit bug open about this?)

I'll also note that Chrome does not actually follow the CSS Text 3 requirements fully; the spec goes on to say:

If the UA chooses to adjust the text, then it must ensure that tab stops continue to line up as required by the white space processing rules.

but in my brief testing, it seems to totally lose track of tab stops in this case.

Comment 6

[shaulgo]

Very interesting, thanks.
Are there any discussions for this? Would be super interesting to understand how they decided on this.
The problem that I am trying to solve and many other devs, is building a rich text editor/viewer that supports justification like in word/docs and other tools.
I don't know what are the considerations of the css spec writers. But I think this is a feature that should be implemented natively in browsers.
Currently, most online text editors either don't support justification or have a "broken" experience with different browsers. While there is wide demand.

Comment 7

[shaulgo]

This is the webkit bug https://bugs.webkit.org/show_bug.cgi?id=84448

Comment 8

[JP]

I have also experienced this bug. In creating a web based page layout tool where typesetting is crucial this bug is easily reproduced when using "text-align: justify" in conjunction with "white-space: pre" or "white-space: pre-wrap."

Comment 9

[Ksenia Berezina [:ksenia]]

This is affecting Word online editor (Microsoft Office 365), where applying "justify" doesn't have any effect on text alignment. Adjusting webcompat priority to P2

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to Jonathan Kew [:jfkthame] from comment #5)

So according to CSS2.1, the Firefox behavior here was correct. This is also reflected in web-platform-tests that Firefox and Safari currently PASS, while Chrome FAILS.

FWIW, Safari fails this test now (possibly due to them having aligned with Chrome on this?)

Firefox is alone in passing that test now (where it sounds like maybe the web depends on us changing behavior?)

Comment 11

[Jonathan Kew [:jfkthame]]

We should probably go ahead and do this, to align with the other browsers, and the spec and test should be updated accordingly.

But I think fixing this will also require us to fix bug 1712703, otherwise the justification will be inaccurate, so marking that as a dependency (and that looks like being the harder part of the task).

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1253840 - patch 1 - Allow text-align:justify to take effect when using white-space:pre-wrap. r=#layout-reviewers

The CSS Text 3 spec allows this (CSS2 didn't), and other browsers
support it, so for better webcompat/interoperability, we should do the
same. Tests will need updating accordingly.

This first patch enables the property to take effect, but the resulting
layout is not yet correct because the (non-collapsed) whitespace at line
wrap positions needs to be forcibly "hung" into the margin for correct
justification; see following patch.

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1253840 - patch 2 - When justifying, the full advance of any trimmable end-of-line whitespace needs to be hung into the margin. r=#layout-reviewers

Depends on D178210

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1253840 - patch 3 - Update test expectations. r=#layout-reviewers

Depends on D178211

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1253840 - patch 4 - Add some new WPT reftests for white-space:pre-wrap + text-align:justify behavior. r=#layout-reviewers

Depends on D178212

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf6eff426d61
patch 1 - Allow text-align:justify to take effect when using white-space:pre-wrap. r=emilio
https://hg.mozilla.org/integration/autoland/rev/7ce24f83240a
patch 2 - When justifying, the full advance of any trimmable end-of-line whitespace needs to be hung into the margin. r=emilio
https://hg.mozilla.org/integration/autoland/rev/816a9266d111
patch 3 - Update test expectations. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/1a47c4ddf44c
patch 4 - Add some new WPT reftests for white-space:pre-wrap + text-align:justify behavior. r=dholbert

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40094 for changes under testing/web-platform/tests

Comment 18

[Serban Stanca [:SerbanS]]

Backed out for causing multiple failures in nsLineLayout.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: Assertion failure: applyState.mGaps.mHandled == applyState.mGaps.mCount (Unprocessed justification gaps), at /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:3207

And also these reftests failures.

• Failure Log
• Failure line: REFTEST ERROR | TEST-UNEXPECTED-FAIL | layout/reftests/text/justification-1.html == layout/reftests/text/justification-1-ref.html | application timed out after 370 seconds with no output

Comment 19

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0397755aaebe
patch 1 - Allow text-align:justify to take effect when using white-space:pre-wrap. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e9ba463e16b8
patch 2 - When justifying, the full advance of any trimmable end-of-line whitespace needs to be hung into the margin. r=emilio
https://hg.mozilla.org/integration/autoland/rev/838411d789f3
patch 3 - Update test expectations. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/c412301a7e7c
patch 4 - Add some new WPT reftests for white-space:pre-wrap + text-align:justify behavior. r=dholbert

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0397755aaebe
https://hg.mozilla.org/mozilla-central/rev/e9ba463e16b8
https://hg.mozilla.org/mozilla-central/rev/838411d789f3
https://hg.mozilla.org/mozilla-central/rev/c412301a7e7c

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88b93df65e7e
Remove .ini file as it's no longer necessary by passing on all platforms. a=test-only

Comment 23

[Cosmin Sabou [:CosminS]]

The land in comment 22 is about these test unexpected-passes on central. The test is skipped on autoland and that's why it didn't turn orange there. https://treeherder.mozilla.org/logviewer?job_id=416520547&repo=autoland&lineNumber=1967

Comment 24

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/88b93df65e7e

Comment 26

[Gérard Talbot]

the web-platform-test mentioned above has been annotated with MAY
(...)
test should be updated accordingly

Jonathan and other Mozilla bugzilla colleagues in this bug report,

The CSS2-based test text/text-align-white-space-003.xht has now been removed from WPT test repository:

https://github.com/web-platform-tests/wpt/pull/40109

and the new tests from phabricator.services.mozilla.com/D178213 have been merged into WPT repository.

Comment 27

[Gérard Talbot]

WPT test results:

https://wpt.fyi/results/css/css-text/white-space?label=experimental&label=master&aligned&q=white-space-pre-wrap-justify-00

Comment 28

[Daniel Holbert [:dholbert]]

Thanks!

(While I'm here: --> closing needinfo from comment 18.)

Comment 29

[Cristian Tuns]

Backed out for causing Bug 1838525
Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/2f3cce1e72fe61e4643225e21ac84ccebe84ed30

Comment 30

[Emilio Cobos Álvarez (:emilio)]

The bug is fixed. Only the annotation needed backout because it was a mistake, it should've been attached to bug 1834081 instead of here.

Comment 31

[Daniel Holbert [:dholbert]]

Restoring the status flags granted from comment 21 which were removed as part of the backout in comment 29.

Per comment 30, only a test-annotation was backed out here; the actual code changes stayed in.