-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-text-3] White space pre wrap end of line #5283
Conversation
Notifying @fantasai, @frivoal, @hakatashi, @kojiishi, @nox, @plinss, and @r12a. (Learn how reviewing works.) |
Originally posted as w3c/csswg-test#1137 (comment) by @syncbot on 17 Oct 2016, 13:15 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @frivoal on 31 Oct 2016, 02:31 UTC: |
Originally posted as w3c/csswg-test#1137 (comment) by @syncbot on 05 Dec 2016, 05:50 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @frivoal on 05 Dec 2016, 06:17 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @syncbot on 15 Jan 2017, 10:02 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @syncbot on 15 Jan 2017, 13:31 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @syncbot on 17 Mar 2017, 10:33 UTC:
|
Originally posted as w3c/csswg-test#1137 (comment) by @frivoal on 17 Mar 2017, 11:29 UTC:
|
@frivoal Can you please you character references where you have trailing whitespace (to make it obvious, regardless of text editor etc.) and then revert the |
Build PASSEDStarted: 2017-08-21 17:08:49 View more information about this build on: |
Chrome (unstable channel)Testing web-platform-tests at revision b56fd61 Unstable results
All results28 tests ran/css/css-text-3/white-space/pre-wrap-001.html
/css/css-text-3/white-space/pre-wrap-002.html
/css/css-text-3/white-space/pre-wrap-003.html
/css/css-text-3/white-space/pre-wrap-004.html
/css/css-text-3/white-space/pre-wrap-005.html
/css/css-text-3/white-space/pre-wrap-006.html
/css/css-text-3/white-space/pre-wrap-007.html
/css/css-text-3/white-space/pre-wrap-008.html
/css/css-text-3/white-space/pre-wrap-009.html
/css/css-text-3/white-space/pre-wrap-010.html
/css/css-text-3/white-space/pre-wrap-011.html
/css/css-text-3/white-space/pre-wrap-012.html
/css/css-text-3/white-space/pre-wrap-013.html
/css/css-text-3/white-space/pre-wrap-014.html
/css/css-text-3/white-space/textarea-pre-wrap-001.html
/css/css-text-3/white-space/textarea-pre-wrap-002.html
/css/css-text-3/white-space/textarea-pre-wrap-003.html
/css/css-text-3/white-space/textarea-pre-wrap-004.html
/css/css-text-3/white-space/textarea-pre-wrap-005.html
/css/css-text-3/white-space/textarea-pre-wrap-006.html
/css/css-text-3/white-space/textarea-pre-wrap-007.html
/css/css-text-3/white-space/textarea-pre-wrap-008.html
/css/css-text-3/white-space/textarea-pre-wrap-009.html
/css/css-text-3/white-space/textarea-pre-wrap-010.html
/css/css-text-3/white-space/textarea-pre-wrap-011.html
/css/css-text-3/white-space/textarea-pre-wrap-012.html
/css/css-text-3/white-space/textarea-pre-wrap-013.html
/css/css-text-3/white-space/textarea-pre-wrap-014.html
|
@gsnedders Travis is failing some tests here ("unstable results"), but I am not sure what that is or how to dig further into it. Can you shed some light? Travis had no complain in the csswg repo. |
@frivoal This seems to be a bit of a random failure in the stability checker (note that Travis checks more than it ever did in csswg-test, for example it'll catch flaky tests most of the time), given it just seems to be throwing an exception half the time. |
@gsnedders Anything I should do about it? Or is it ok to merge despite this check failing (once the rest of the review is done)? |
5c1bcae
to
9345aca
Compare
These tests are now available on w3c-test.org |
Rebased the branch to get the fix for #5330 |
@gsnedders I cannot parse this sentence of yours.
Also, what is travis complaining about. I don't understand this thing about instability. |
Probably jugglinmike/chrome-screenshot-race#1. |
Sauce (safari)Testing web-platform-tests at revision 56ea448 All results28 tests ran/css/css-text-3/white-space/pre-wrap-001.html
/css/css-text-3/white-space/pre-wrap-002.html
/css/css-text-3/white-space/pre-wrap-003.html
/css/css-text-3/white-space/pre-wrap-004.html
/css/css-text-3/white-space/pre-wrap-005.html
/css/css-text-3/white-space/pre-wrap-006.html
/css/css-text-3/white-space/pre-wrap-007.html
/css/css-text-3/white-space/pre-wrap-008.html
/css/css-text-3/white-space/pre-wrap-009.html
/css/css-text-3/white-space/pre-wrap-010.html
/css/css-text-3/white-space/pre-wrap-011.html
/css/css-text-3/white-space/pre-wrap-012.html
/css/css-text-3/white-space/pre-wrap-013.html
/css/css-text-3/white-space/pre-wrap-014.html
/css/css-text-3/white-space/textarea-pre-wrap-001.html
/css/css-text-3/white-space/textarea-pre-wrap-002.html
/css/css-text-3/white-space/textarea-pre-wrap-003.html
/css/css-text-3/white-space/textarea-pre-wrap-004.html
/css/css-text-3/white-space/textarea-pre-wrap-005.html
/css/css-text-3/white-space/textarea-pre-wrap-006.html
/css/css-text-3/white-space/textarea-pre-wrap-007.html
/css/css-text-3/white-space/textarea-pre-wrap-008.html
/css/css-text-3/white-space/textarea-pre-wrap-009.html
/css/css-text-3/white-space/textarea-pre-wrap-010.html
/css/css-text-3/white-space/textarea-pre-wrap-011.html
/css/css-text-3/white-space/textarea-pre-wrap-012.html
/css/css-text-3/white-space/textarea-pre-wrap-013.html
/css/css-text-3/white-space/textarea-pre-wrap-014.html
|
Chrome (unstable)Testing web-platform-tests at revision 56ea448 All results28 tests ran/css/css-text-3/white-space/pre-wrap-001.html
/css/css-text-3/white-space/pre-wrap-002.html
/css/css-text-3/white-space/pre-wrap-003.html
/css/css-text-3/white-space/pre-wrap-004.html
/css/css-text-3/white-space/pre-wrap-005.html
/css/css-text-3/white-space/pre-wrap-006.html
/css/css-text-3/white-space/pre-wrap-007.html
/css/css-text-3/white-space/pre-wrap-008.html
/css/css-text-3/white-space/pre-wrap-009.html
/css/css-text-3/white-space/pre-wrap-010.html
/css/css-text-3/white-space/pre-wrap-011.html
/css/css-text-3/white-space/pre-wrap-012.html
/css/css-text-3/white-space/pre-wrap-013.html
/css/css-text-3/white-space/pre-wrap-014.html
/css/css-text-3/white-space/textarea-pre-wrap-001.html
/css/css-text-3/white-space/textarea-pre-wrap-002.html
/css/css-text-3/white-space/textarea-pre-wrap-003.html
/css/css-text-3/white-space/textarea-pre-wrap-004.html
/css/css-text-3/white-space/textarea-pre-wrap-005.html
/css/css-text-3/white-space/textarea-pre-wrap-006.html
/css/css-text-3/white-space/textarea-pre-wrap-007.html
/css/css-text-3/white-space/textarea-pre-wrap-008.html
/css/css-text-3/white-space/textarea-pre-wrap-009.html
/css/css-text-3/white-space/textarea-pre-wrap-010.html
/css/css-text-3/white-space/textarea-pre-wrap-011.html
/css/css-text-3/white-space/textarea-pre-wrap-012.html
/css/css-text-3/white-space/textarea-pre-wrap-013.html
/css/css-text-3/white-space/textarea-pre-wrap-014.html
|
Sauce (MicrosoftEdge)Testing web-platform-tests at revision 56ea448 All results28 tests ran/css/css-text-3/white-space/pre-wrap-001.html
/css/css-text-3/white-space/pre-wrap-002.html
/css/css-text-3/white-space/pre-wrap-003.html
/css/css-text-3/white-space/pre-wrap-004.html
/css/css-text-3/white-space/pre-wrap-005.html
/css/css-text-3/white-space/pre-wrap-006.html
/css/css-text-3/white-space/pre-wrap-007.html
/css/css-text-3/white-space/pre-wrap-008.html
/css/css-text-3/white-space/pre-wrap-009.html
/css/css-text-3/white-space/pre-wrap-010.html
/css/css-text-3/white-space/pre-wrap-011.html
/css/css-text-3/white-space/pre-wrap-012.html
/css/css-text-3/white-space/pre-wrap-013.html
/css/css-text-3/white-space/pre-wrap-014.html
/css/css-text-3/white-space/textarea-pre-wrap-001.html
/css/css-text-3/white-space/textarea-pre-wrap-002.html
/css/css-text-3/white-space/textarea-pre-wrap-003.html
/css/css-text-3/white-space/textarea-pre-wrap-004.html
/css/css-text-3/white-space/textarea-pre-wrap-005.html
/css/css-text-3/white-space/textarea-pre-wrap-006.html
/css/css-text-3/white-space/textarea-pre-wrap-007.html
/css/css-text-3/white-space/textarea-pre-wrap-008.html
/css/css-text-3/white-space/textarea-pre-wrap-009.html
/css/css-text-3/white-space/textarea-pre-wrap-010.html
/css/css-text-3/white-space/textarea-pre-wrap-011.html
/css/css-text-3/white-space/textarea-pre-wrap-012.html
/css/css-text-3/white-space/textarea-pre-wrap-013.html
/css/css-text-3/white-space/textarea-pre-wrap-014.html
|
ac881e7
to
db38309
Compare
lint.whitelist
Outdated
@@ -481,6 +481,10 @@ TRAILING WHITESPACE: css/vendor-imports/mozilla/mozilla-central-reftests/css21/p | |||
TRAILING WHITESPACE: css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-001-rtl-reverse.xhtml | |||
TRAILING WHITESPACE: css/vendor-imports/mozilla/mozilla-central-reftests/multicol3/moz-multicol3-column-balancing-break-inside-avoid-1.html | |||
TRAILING WHITESPACE: css/vendor-imports/mozilla/mozilla-central-reftests/multicol3/moz-multicol3-column-balancing-break-inside-avoid-1-ref.html | |||
TRAILING WHITESPACE: css/css-text-3/white-space/textarea-pre-wrap-011.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use HTML entities at the end of the line instead of whitelisting it? It makes it much easier to read the test (v. trying to read invisible characters!).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
43caaf4
to
28f2532
Compare
@FremyCompany Can you give a look at these text tests? |
@plehegar ping. Can you have a look at why travis is failing please? |
@frivoal if you rebase you'll find it's now an allowed failure |
28f2532
to
f8b55c7
Compare
@gsnedders The rebase worked. Thanks for the heads up. @FremyCompany There's no longer anything standing between you and a code review :) |
@frivoal on it |
<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-phase-2"> | ||
<link rel="help" href="https://drafts.csswg.org/css-text-3/#overflow-wrap-property"> | ||
<link rel="match" href="reference/pre-wrap-001-ref.html"> | ||
<meta name="assert" content="When the hite-space property is set to pre-wrap, preserved white space at the end of the line must hang or be collapsed, and must not cause preceeding content to be wrapped."> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/hite-space/white-space
<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-phase-2"> | ||
<link rel="help" href="https://drafts.csswg.org/css-text-3/#overflow-wrap-property"> | ||
<link rel="match" href="reference/pre-wrap-001-ref.html"> | ||
<meta name="assert" content="When the hite-space property is set to pre-wrap, preserved white space at the end of the line must hang or be collapsed, and must not cause preceeding content to be wrapped."> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where in the spec is that defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In https://drafts.csswg.org/css-text-3/#white-space-phase-2 subpoint 4
<link rel="help" href="https://drafts.csswg.org/css-text-3/#white-space-phase-2"> | ||
<link rel="help" href="https://drafts.csswg.org/css-text-3/#overflow-wrap-property"> | ||
<link rel="match" href="reference/textarea-pre-wrap-001-ref.html"> | ||
<meta name="assert" content="When the hite-space property is set to pre-wrap, preserved white space at the end of the line must hang or be collapsed, and must not cause preceeding content to be wrapped in a textarea."> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Build PASSEDStarted: 2017-09-01 01:13:15 Failing Jobs
View more information about this build on: |
Originally posted as w3c/csswg-test#1137 by @frivoal on 17 Oct 2016, 13:14 UTC: