Closed
Bug 955857
Opened 11 years ago
Closed 9 years ago
accept the overflow-wrap property
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: sjw+bugzilla, Assigned: wisniewskit)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 2 obsolete files)
9.95 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
18.48 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
"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
Blocks: css-text-3, css3test
Depends on: 587438
Updated•11 years ago
|
Severity: normal → enhancement
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.
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
Updated•10 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 3•9 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)
Assignee | ||
Comment 4•9 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]
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.
Attachment #8755647 -
Flags: review?(dbaron) → review+
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+
Attachment #8755648 -
Flags: review?(dbaron) → review-
Assignee | ||
Comment 7•9 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)
Assignee | ||
Comment 8•9 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]
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.
Attachment #8755664 -
Flags: review?(dbaron) → review+
Attachment #8755666 -
Flags: review+
Flags: needinfo?(dbaron)
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.
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.
Assignee | ||
Comment 13•9 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.
Assignee: nobody → wisniewskit
Comment 15•9 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
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 16•9 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
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•