Skip to content
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

Layout 2020: Implement basic white-space: pre support #26447

Merged
merged 2 commits into from Jul 29, 2020

Conversation

jdm
Copy link
Member

@jdm jdm commented May 6, 2020

With these changes <pre> and <br> preserve spaces and force line breaks appropriately.


@highfive
Copy link

highfive commented May 6, 2020

Heads up! This PR modifies the following files:

  • @emilio: components/style/properties/longhands/inherited_text.mako.rs

@highfive
Copy link

highfive commented May 6, 2020

warning Warning warning

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

@jdm jdm changed the title Implement basic white-space: pre support for layout 2020. Layout 2020: Implement basic white-space: pre support May 6, 2020
@jdm
Copy link
Member Author

jdm commented May 6, 2020

@bors-servo try=wpt-2020

@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2020

Trying commit f98c164 with merge b73c92c...

bors-servo added a commit that referenced this issue May 6, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented May 6, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Member

CYBAI commented May 6, 2020

  ▶ PASS [expected FAIL] /css/CSS2/abspos/hypothetical-inline-alone-on-second-line.html
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-004.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-018.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-016.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-applies-to-003.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-052.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-013.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-017.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-047.xht
  ▶ PASS [expected FAIL] /css/CSS2/text/white-space-processing-046.xht
  ▶ PASS [expected FAIL] /css/CSS2/colors/color-applies-to-008.xht
  ▶ PASS [expected FAIL] /css/CSS2/colors/color-applies-to-015.xht
  ▶ PASS [expected FAIL] /css/CSS2/css1/c562-white-sp-000.xht
  ▶ PASS [expected FAIL] /css/CSS2/floats-clear/float-003.xht
  ▶ FAIL [expected PASS] /css/CSS2/floats/floats-placement-vertical-004.xht
  │   → /css/CSS2/floats/floats-placement-vertical-004.xht bbb7c1113b094885a70a23b94b54387b547f60ee
  └   → /css/CSS2/floats/floats-placement-vertical-004-ref.xht 6c8c6d5333ff6664251cc1c0563d4977abbad2d7
  ▶ FAIL [expected PASS] /css/CSS2/generated-content/content-173.xht
  │   → /css/CSS2/generated-content/content-173.xht 8f4c7177ee0cd4f67987eec19c95880a40601950
  └   → /css/CSS2/generated-content/content-173-ref.xht 56ac0bda6b15b70637b89458cba3a119e0ac33ef
  ▶ PASS [expected FAIL] /css/CSS2/generated-content/content-171.xht
  ▶ FAIL [expected PASS] /css/CSS2/generated-content/content-white-space-002.xht
  │   → /css/CSS2/generated-content/content-white-space-002.xht f03887f986bf78d79db67efaf8e298acf116536e
  └   → /css/CSS2/generated-content/content-white-space-002-ref.html 7c1c92ca33bce0163e6ad166fd8536faa472aec8
  ▶ PASS [expected FAIL] /css/CSS2/generated-content/content-newline-001.xht
  ▶ PASS [expected FAIL] /css/CSS2/margin-padding-clear/padding-percentage-inherit-001.xht
  ▶ PASS [expected FAIL] /css/CSS2/normal-flow/inline-replaced-width-006.xht
  ▶ PASS [expected FAIL] /css/CSS2/normal-flow/inline-replaced-width-001.xht
  ▶ PASS [expected FAIL] /css/CSS2/normal-flow/block-replaced-width-006.xht
  ▶ PASS [expected FAIL] /css/CSS2/normal-flow/inlines-016.xht
  ▶ PASS [expected FAIL] /css/CSS2/positioning/absolute-non-replaced-height-008.xht
  ▶ PASS [expected FAIL] /css/CSS2/positioning/position-static-001.xht
  ▶ FAIL [expected PASS] /css/CSS2/tables/table-anonymous-objects-009.xht
  │   → /css/CSS2/tables/table-anonymous-objects-009.xht f812e682f2e42e4f356e8005957de1b170b38e09
  └   → /css/CSS2/tables/reference/no_red_antialiasing_a_bc_d-ref.xht aa5f2e2c376bd6b8862c2d8d71dc57d0460400a7
  ▶ PASS [expected FAIL] /css/CSS2/visuren/anonymous-boxes-001b.xht
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-size-030.html
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-size-031.html
  ▶ PASS [expected FAIL] /css/css-text-decor/text-decoration-subelements-001.html
  ▶ PASS [expected FAIL] /_mozilla/css/br.html
  ▶ PASS [expected FAIL] /_mozilla/css/input_whitespace.html
  ▶ PASS [expected FAIL] /_mozilla/css/line_breaking_whitespace_collapse_a.html
  ▶ PASS [expected FAIL] /_mozilla/css/word-break-keep-all-007.htm
  ▶ PASS [expected FAIL] /_mozilla/css/word-break-keep-all-005.htm
  ▶ FAIL [expected PASS] /_mozilla/css/white-space-pre-line.htm
  │   → /_mozilla/css/white-space-pre-line.htm 658e4137cba65c258ad44fafa89dfcc9907c4706
  └   → /_mozilla/css/white-space-pre-line-ref.htm c121e7f51e0d9cf70f18ca6cefc6a59e76ed1344
  ▶ PASS [expected FAIL] /_mozilla/css/word-break-keep-all-006.htm
  ▶ PASS [expected FAIL] /_mozilla/css/white_space_intrinsic_sizes_a.html
  ▶ FAIL [expected PASS] /css/CSS2/floats/float-no-content-beside-001.html
  │   → /css/CSS2/floats/float-no-content-beside-001.html 89b7ac526ce2efd4ec1aaa40dc36c3fb382f207a
  └   → /css/CSS2/floats/float-no-content-beside-001-ref.html e6b356c6fb1c66d277610b9a2d10dd8c1ed17e3f
  ▶ PASS [expected FAIL] /_mozilla/css/whitespace_pre.html
  ▶ FAIL [expected PASS] /css/CSS2/tables/table-anonymous-objects-010.xht
  │   → /css/CSS2/tables/table-anonymous-objects-010.xht fb8f5882670545314403c098ec9661e022ae311b
  └   → /css/CSS2/tables/reference/no_red_antialiasing_a_bc_d-ref.xht aa5f2e2c376bd6b8862c2d8d71dc57d0460400a7
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-size-028.html
  ▶ PASS [expected FAIL] /_mozilla/css/many_brs_a.html

@bors-servo
Copy link
Contributor

bors-servo commented Jun 6, 2020

The latest upstream changes (presumably #26805) made this pull request unmergeable. Please resolve the merge conflicts.

@nox
Copy link
Member

nox commented Jun 8, 2020

This seems ok to me but it needs a rebase.

@nox
Copy link
Member

nox commented Jun 25, 2020

What's the status on this, boss?

@jdm
Copy link
Member Author

jdm commented Jun 25, 2020

The status is that I investigated the test failures and discovered that supporting break elements made pre elements render incorrectly, which led to #26754. I haven't figured out if there's a more-optimal solution yet.

@jdm
Copy link
Member Author

jdm commented Jul 24, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2020

Trying commit 7ee9b25 with merge dc737cc...

bors-servo added a commit that referenced this issue Jul 24, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@jdm
Copy link
Member Author

jdm commented Jul 24, 2020

The hard drive that contained all of my investigations for #26754 died, and I've been focusing on FxR release management recently. I think the changes in this PR are an improvement over the status quo and don't make the affected code any more difficult to reason about, so we should probably proceed with getting this landed.

@jdm
Copy link
Member Author

jdm commented Jul 24, 2020

@bors-servo try=wpt-2020 retry

bors-servo added a commit that referenced this issue Jul 24, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2020

Trying commit 7ee9b25 with merge 378993a...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Jul 28, 2020

@bors-servo try=wpt-2020

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

Trying commit 260347e with merge 86383d9...

bors-servo added a commit that referenced this issue Jul 28, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2020

☀️ Test successful - status-taskcluster
State: approved= try=True

@nox
Copy link
Member

nox commented Jul 29, 2020

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

📌 Commit 260347e has been approved by nox

bors-servo added a commit that referenced this issue Jul 29, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

Testing commit 260347e with merge 89d1cf2...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Member

CYBAI commented Jul 29, 2020

@bors-servo retry

  ▶ Unexpected subtest result in /html/canvas/element/fill-and-stroke-styles/2d.pattern.paint.repeat.coord2.html:
  │ FAIL [expected PASS] Canvas test: 2d.pattern.paint.repeat.coord2
  │   → assert_equals: Green channel of the pixel at (1, 1) expected 255 but got 0
  │ 
  │ _assertPixel@http://web-platform.test:8000/html/canvas/resources/canvas-tests.js:39:18
  │ @http://web-platform.test:8000/html/canvas/element/fill-and-stroke-styles/2d.pattern.paint.repeat.coord2.html:27:13
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1977:25
  │ _addTest/</<@http://web-platform.test:8000/html/canvas/resources/canvas-tests.js:66:15
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1977:25
  └ _addTest/<@http://web-platform.test:8000/html/canvas/resources/canvas-tests.js:63:11

@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

Testing commit 260347e with merge 544c379...

bors-servo added a commit that referenced this issue Jul 29, 2020
Layout 2020: Implement basic white-space: pre support

With these changes `<pre>` and `<br>` preserve spaces and force line breaks appropriately.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26440
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member Author

jdm commented Jul 29, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

Testing commit 260347e with merge 6a3c3a4...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 29, 2020

☀️ Test successful - status-taskcluster
Approved by: nox
Pushing 6a3c3a4 to master...

@bors-servo bors-servo merged commit 6a3c3a4 into servo:master Jul 29, 2020
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants