Implement `word-break: keep-all` (#9673) #13414

flacerdk · 2016-09-25T13:29:45Z

Implement the keep-all value for the word-break property, as specified in CSS.

The relevant CSSWG tests (in tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.)

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #9673.

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2016-09-25T13:29:50Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.

highfive · 2016-09-25T13:29:51Z

Heads up! This PR modifies the following files:

@bholley: components/style/properties/longhand/inherited_text.mako.rs

highfive · 2016-09-25T13:29:52Z

Warning

These commits modify style, layout, and gfx code, but no tests are modified. Please consider adding a test!

stshine · 2016-09-25T13:43:37Z

@bors-servo try

bors-servo · 2016-09-25T13:43:39Z

⌛ Trying commit 0c80d84 with merge 6675ea8...


        Auto merge of #13414 - flacerdk:master, r=<try>

Implement `word-break: keep-all` (#9673)  Implement the `keep-all` value for the `word-break` property, as specified in [CSS](https://drafts.csswg.org/css-text-3/#word-break-property). The relevant CSSWG tests (in `tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm`) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.) ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9673.  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13414)

bors-servo · 2016-09-25T13:54:13Z

💔 Test failed - linux-dev

jdm · 2016-09-25T14:43:28Z

@flacerdk Could you be more specific about the JS that's not working and how it is not doing so?

jdm · 2016-09-25T14:44:38Z

http://build.servo.org/builders/linux-rel/builds/3348/steps/test_3/logs/stdio shows a lot of "FAIL [expected PASS]" results, but also a bunch of "PASS [expected FAIL]" for /css-text-3_dev/html/css3-text-line-break-jazh-* tests.

jdm · 2016-09-25T14:48:33Z

Oh, I bet you're hitting #12939, since the tests are checking offsetWidth of a span :<

jdm · 2016-09-25T14:48:39Z

r? @mbrubeck

flacerdk · 2016-09-25T16:44:37Z

Indeed that seems to be the problem with the word-break tests.

I had overlooked the line-break ones; I'll take a look into that now.

flacerdk · 2016-09-26T10:49:06Z

Oops, looks like I messed up with rebasing and accidentally closed this.

The line-break tests should pass now. There are some other tests that are not passing on my machine, but they also seem to be failing when I build HEAD, so hopefully it's just an issue with my local setup.

emilio · 2016-09-26T11:12:02Z

@bors-servo: try

bors-servo · 2016-09-26T11:12:05Z

⌛ Trying commit f16cff4 with merge 4c6cf8a...


        Auto merge of #13414 - flacerdk:master, r=<try>

Implement `word-break: keep-all` (#9673)  Implement the `keep-all` value for the `word-break` property, as specified in [CSS](https://drafts.csswg.org/css-text-3/#word-break-property). The relevant CSSWG tests (in `tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm`) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.) ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9673.  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13414)

bors-servo · 2016-09-26T12:57:16Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

emilio · 2016-09-26T15:36:59Z

I'm surprised no text expectations change, can you write a reftest that tests this if there isn't, please? Thanks! :)

jdm · 2016-09-26T16:18:43Z

Note: docs on writing tests exist.

flacerdk · 2016-09-26T16:27:26Z

I'll work on that; I suppose I can write versions of the currently existing tests that don't depend on #12939 being fixed.

jdm · 2016-09-26T16:30:11Z

Certainly I don't think it's necessary to duplicate all of them, but a couple representative ones demonstrating the correct functioning of keep-all would be valuable.

mbrubeck · 2016-09-27T22:55:43Z

@bors-servo retry

#12654

bors-servo · 2016-09-28T03:41:35Z

⌛ Testing commit fb66abb with merge 8f451cf...


        Auto merge of #13414 - flacerdk:master, r=mbrubeck

Implement `word-break: keep-all` (#9673)  Implement the `keep-all` value for the `word-break` property, as specified in [CSS](https://drafts.csswg.org/css-text-3/#word-break-property). The relevant CSSWG tests (in `tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm`) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.) ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9673.  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13414)

bors-servo · 2016-09-28T03:43:18Z

💔 Test failed - mac-dev-unit

KiChjang · 2016-09-28T04:01:05Z

@bors-servo retry

flacerdk · 2016-09-29T17:44:48Z

Did the buildbot die?

mbrubeck · 2016-09-29T18:16:14Z

@bors-servo r+ retry

bors-servo · 2016-09-29T18:16:15Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: #13323

bors-servo · 2016-09-29T18:16:15Z

📌 Commit fb66abb has been approved by mbrubeck

bors-servo · 2016-09-29T20:29:50Z

⌛ Testing commit fb66abb with merge 2e65d8a...


        Auto merge of #13414 - flacerdk:master, r=mbrubeck

Implement `word-break: keep-all` (#9673)  Implement the `keep-all` value for the `word-break` property, as specified in [CSS](https://drafts.csswg.org/css-text-3/#word-break-property). The relevant CSSWG tests (in `tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm`) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.) ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9673.  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13414)

bors-servo · 2016-09-29T21:21:33Z

💔 Test failed - linux-rel

jdm · 2016-09-29T22:50:06Z

@bors-servo: retry

#13479


        Auto merge of #13414 - flacerdk:master, r=mbrubeck

Implement `word-break: keep-all` (#9673)  Implement the `keep-all` value for the `word-break` property, as specified in [CSS](https://drafts.csswg.org/css-text-3/#word-break-property). The relevant CSSWG tests (in `tests/wpt/css-tests/css-text-3_dev/html/word-break-keep-all-*.htm`) do not currently pass. As far as I can tell, this is because the tests use some JavaScript code that is not working properly. (But then, it seems that most tests in this directory are failing at the moment. I'm not sure what can be done here for now.) ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #9673.  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13414)

bors-servo · 2016-09-30T00:20:56Z

⌛ Testing commit fb66abb with merge c944234...

bors-servo · 2016-09-30T01:21:30Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

highfive assigned jdm Sep 25, 2016

highfive added the S-awaiting-review label Sep 25, 2016

highfive added the S-tests-failed label Sep 25, 2016

highfive assigned mbrubeck and unassigned jdm Sep 25, 2016

flacerdk closed this Sep 26, 2016

flacerdk force-pushed the flacerdk:master branch from 0c80d84 to d00639c Sep 26, 2016

highfive removed the S-tests-failed label Sep 26, 2016

flacerdk reopened this Sep 26, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 27, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 28, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 28, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 29, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Sep 29, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Sep 30, 2016

bors-servo merged commit fb66abb into servo:master Sep 30, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Sep 30, 2016

servo / servo

Implement `word-break: keep-all` (#9673) #13414

Implement `word-break: keep-all` (#9673) #13414

flacerdk commented Sep 25, 2016 •

edited by larsbergstrom

highfive commented Sep 25, 2016

highfive commented Sep 25, 2016

highfive commented Sep 25, 2016

stshine commented Sep 25, 2016

bors-servo commented Sep 25, 2016

bors-servo commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

flacerdk commented Sep 25, 2016

flacerdk commented Sep 26, 2016

emilio commented Sep 26, 2016

bors-servo commented Sep 26, 2016

bors-servo commented Sep 26, 2016

emilio commented Sep 26, 2016

jdm commented Sep 26, 2016

flacerdk commented Sep 26, 2016

jdm commented Sep 26, 2016

mbrubeck commented Sep 27, 2016

bors-servo commented Sep 28, 2016

bors-servo commented Sep 28, 2016

KiChjang commented Sep 28, 2016

flacerdk commented Sep 29, 2016

mbrubeck commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

jdm commented Sep 29, 2016

bors-servo commented Sep 30, 2016

bors-servo commented Sep 30, 2016

servo / servo

Join GitHub today

Implement `word-break: keep-all` (#9673) #13414

Implement `word-break: keep-all` (#9673) #13414

Conversation

flacerdk commented Sep 25, 2016 • edited by larsbergstrom

highfive commented Sep 25, 2016

highfive commented Sep 25, 2016

highfive commented Sep 25, 2016

stshine commented Sep 25, 2016

bors-servo commented Sep 25, 2016

bors-servo commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

jdm commented Sep 25, 2016

flacerdk commented Sep 25, 2016

flacerdk commented Sep 26, 2016

emilio commented Sep 26, 2016

bors-servo commented Sep 26, 2016

bors-servo commented Sep 26, 2016

emilio commented Sep 26, 2016

jdm commented Sep 26, 2016

flacerdk commented Sep 26, 2016

jdm commented Sep 26, 2016

mbrubeck commented Sep 27, 2016

bors-servo commented Sep 28, 2016

bors-servo commented Sep 28, 2016

KiChjang commented Sep 28, 2016

flacerdk commented Sep 29, 2016

mbrubeck commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

bors-servo commented Sep 29, 2016

jdm commented Sep 29, 2016

bors-servo commented Sep 30, 2016

bors-servo commented Sep 30, 2016

flacerdk commented Sep 25, 2016 •

edited by larsbergstrom