accept the overflow-wrap property

RESOLVED FIXED in Firefox 49



5 years ago
3 years ago


(Reporter: sjw, Assigned: wisniewskit)


(Depends on: 1 bug, Blocks: 2 bugs, {dev-doc-complete})

Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)



(2 attachments, 2 obsolete attachments)



5 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’."


5 years ago
Blocks: 104960, 913153
Depends on: 587438
Severity: normal → enhancement

Comment 1

5 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.
Severity: enhancement → normal
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.
Severity: normal → enhancement
See Also: → bug 959735
Keywords: dev-doc-needed

Comment 3

3 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.
Attachment #8755647 - Flags: review?(dbaron)

Comment 4

3 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.
Attachment #8755648 - Flags: review?(dbaron)
Comment on attachment 8755647 [details] [diff] [review]

>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 run once the patch is revised before it lands.  needinfo? me and I can push it for you.
Attachment #8755647 - Flags: review?(dbaron) → review+
Comment on attachment 8755648 [details] [diff] [review]

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+
Attachment #8755648 - Flags: review?(dbaron) → review-

Comment 7

3 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.
Attachment #8755648 - Attachment is obsolete: true
Attachment #8755664 - Flags: review?(dbaron)

Comment 8

3 years ago
Here's the revision with the re-ordering.

needinfoing you as requested.
Attachment #8755647 - Attachment is obsolete: true
Flags: needinfo?(dbaron)
Comment on attachment 8755664 [details] [diff] [review]

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.
Attachment #8755664 - Flags: review?(dbaron) → review+
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.

Comment 13

3 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).
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.

Comment 15

3 years ago
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
I've moved the word-wrap page to
[And adapted it of course]
and updated:
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.