sjw Reporter
	Description • 11 years ago

"For legacy reasons, UAs must treat ‘word-wrap’ as an alternate name for the ‘overflow-wrap’ property, as if it were a shorthand of ‘overflow-wrap’."
http://dev.w3.org/csswg/css-text/#overflow-wrap

	Florian Bender
	Comment 1 • 11 years ago

This is a Gecko bug wrt spec compliance. As this is part of CSS3-Text (and not a new spec), I reset the severity to normal. IMHO, only if this concerned a new spec that has no prior implementation in Gecko, the "enhancement" value would be justified.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 2 • 11 years ago

It's a new feature; word-wrap predates the spec, but overflow-wrap does not.  It's still trivial, and we should do it when the spec reaches CR.

	Thomas Wisniewski Assignee
	Comment 3 • 8 years ago

Here's a patch. I'm not sure if I went too far with the renames/comment changes here, but they seemed reasonable given that overflow-wrap is now considered the property proper in the spec.

	Thomas Wisniewski Assignee
	Comment 4 • 8 years ago

Here are the tests. I'm not sure if the file names for the new refrests should be re-numbered or kept to match the corresponding word-wrap tests they were copied from.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 5 • 8 years ago

Comment on attachment 8755647 [details] [diff] [review]
955857-add-support-for-css-overflow-wrap.diff

>diff --git a/layout/style/nsComputedDOMStylePropertyList.h b/layout/style/nsComputedDOMStylePropertyList.h

I believe the list in this file needs to be sorted alphabetically, or it will fail tests.

r=dbaron with that fixed


I think we're probably ok not having a pref for this (which would actually be nontrivial, since we'd need a pref that controlled the main property but not the alias).

This should get a https://wiki.mozilla.org/ReleaseEngineering/TryServer run once the patch is revised before it lands.  needinfo? me and I can push it for you.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 6 • 8 years ago

Comment on attachment 8755648 [details] [diff] [review]
955857-add-tests-for-css-overflow-wrap.diff

overflowwrap-04 and overflowwrap-05 should use overflowWrap rather than wordWrap
.

There's also no need to copy the -ref files (even though you did change some of them), but you need to fix the manifest accordingly.  For the ones where the -ref file uses overflow-wrap (4, 5, 8, 9, I think), you could do an == test between the two tests instead of having a second copy of the reference.

r=dbaron with those things fixed, although I should probably look at the revisions, so not marking as review+

	Thomas Wisniewski Assignee
	Comment 7 • 8 years ago

It was only tests 4 and 9, after a second look. Turns out that 8 was really testing the output of overflow:scroll against the results of word-wrap, so I don't suspect it's worth duping that one. The rest I adjusted as you suggested.

	Thomas Wisniewski Assignee
	Comment 8 • 8 years ago

Here's the revision with the re-ordering.

needinfoing you as requested.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 9 • 8 years ago

Comment on attachment 8755664 [details] [diff] [review]
955857-add-tests-for-css-overflow-wrap.diff

r=dbaron, although I'm not sure why I suggested comparing 4 and 9 to the test rather than reference; they may as well compare to the reference like the others.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 10 • 8 years ago

Try push at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=645e7269bc3c

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 11 • 8 years ago

One failure so far that seems related, in mochitest-devtools-8:

INFO TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_completion-new-property_02.js | Number of suggestions match - Got 17, expected 16

Given that it's testing the devtools autocompletion for the number of properties that start with "o", I think it's just fine to bump the number to 17.  I'll test that locally without bothering with another try run.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 12 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecd39abf2cdb7f988fe7cc1af2f670bd086f309d
Bug 955857 - Replace CSS word-wrap with overflow-wrap, and add it back as a CSS_PROP_ALIAS.  r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/47a2042ad9cef82c21ff09cbe52ec764185574dd
Bug 955857 - Add tests for overflow-wrap.  r=dbaron

https://hg.mozilla.org/integration/mozilla-inbound/rev/986a22158836bddedd8ea6dda4134ce20984ab2c
Bug 955857 - Adjust test to expect that there are now 17 CSS properties beginning with 'o' rather than 16.  No review.

	Thomas Wisniewski Assignee
	Comment 13 • 8 years ago

Thanks for the try run. It seems to have completed now, and I can't see the other two failures being related to this patchset. (Also thanks for fixing the other failure).

Let me know if there's anything else I should deal with before this can be considered for landing (whenever that becomes appropriate).

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 14 • 8 years ago

comment 12 (which is script-generated) says I already landed it on mozilla-inbound, which is merged to mozilla-central once or twice per day.

	Carsten Book [:Tomcat]
	Comment 15 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/ecd39abf2cdb
https://hg.mozilla.org/mozilla-central/rev/47a2042ad9ce
https://hg.mozilla.org/mozilla-central/rev/986a22158836

	Jean-Yves Perrier [:teoli]
	Comment 16 • 8 years ago

I've moved the word-wrap page to 
https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap
[And adapted it of course]
and updated:
https://developer.mozilla.org/en-US/Firefox/Releases/49#CSS