Partial text decoration support for layout 2020 #25888

ferjm · 2020-03-04T12:21:55Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #25166
There are tests for these changes

highfive · 2020-03-04T12:22:02Z

Heads up! This PR modifies the following files:

@jgraham: tests/wpt/include-layout-2020.ini
@emilio: components/style/properties/shorthands/text.mako.rs, components/style/properties/longhands/text.mako.rs

ferjm · 2020-03-04T12:22:15Z

@bors-servo try=layout-2020

bors-servo · 2020-03-04T12:22:17Z

🙁 There is no try chooser layout-2020 for this repo, try one of: linux, mac, windows, wpt, wpt-2020, wpt-mac, wpt-android, android, magicleap, arm

ferjm · 2020-03-04T12:22:43Z

@bor-servo try=wpt-2020

emilio · 2020-03-04T13:09:19Z

components/layout_2020/display_list/mod.rs

+        let text_decorations = fragment
+            .parent_style
+            .get_inherited_text()
+            .text_decorations_in_effect;


It would be good to not use the text_decorations_in_effect mechanism, because it's pretty busted, see https://searchfox.org/mozilla-central/rev/d5b34cc8872177d3ee071e06f787c2a14268595b/servo/components/style/style_adjuster.rs#550

CYBAI · 2020-03-04T13:18:52Z

@bors-servo try=wpt-2020

bors-servo · 2020-03-04T13:18:58Z

⌛ Trying commit f7fd0f9 with merge 63167b9...


        Auto merge of #25888 - ferjm:text.decoration.2020, r=<try>

[WIP] Text decoration support for layout 2020 - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25166 - [X] There are tests for these changes

bors-servo · 2020-03-04T13:55:12Z

💔 Test failed - status-taskcluster

CYBAI · 2020-03-04T14:11:45Z

👀

  ▶ FAIL [expected PASS] /css/CSS2/generated-content/content-070.xht
  │   → /css/CSS2/generated-content/content-070.xht 520d8e614f1c2732a4d76fa0a46b43dc23bd2625
  └   → /css/CSS2/generated-content/content-070-ref.html 31ea9caaabf2e1460d2a7f3e2d5b8e45ca608e5d
  ▶ PASS [expected FAIL] /_mozilla/css/text_decoration_smoke_a.html
  ▶ PASS [expected FAIL] /_mozilla/css/text_decoration_underline_subpx_a.html

ferjm · 2020-03-05T08:52:53Z

@bors-servo try=wpt-2020

bors-servo · 2020-03-05T08:52:59Z

⌛ Trying commit d8210c4 with merge b6a9f98...


        Auto merge of #25888 - ferjm:text.decoration.2020, r=<try>

[WIP] Text decoration support for layout 2020 - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25166 - [X] There are tests for these changes

bors-servo · 2020-03-05T09:23:15Z

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

ferjm · 2020-03-05T16:18:54Z

r? @emilio

emilio

you need to walk up the fragment tree to figure out the actually-in-effect decorations.

nox · 2020-03-06T11:38:17Z

you need to walk up the fragment tree to figure out the actually-in-effect decorations.

I think we can accumulate when we recurse down the fragment tree, right?

emilio · 2020-03-06T13:34:22Z

Yeah, that should work too of course.

nox · 2020-03-10T08:31:51Z

That approach look a bit unfortunate to me. Can't we just have a variable of type TextDecorationLine when creating the fragment tree, instead of having to do additional loops over already constructed fragments? I think the current algo leads to a quadratic complexity.


        Add support for text-decoration-style


        Do not use buggy text_decorations_in_effect


        Propagate text decoration where needed


        Update test expectations with text-decoration support

ferjm · 2020-03-23T12:56:59Z

r? ping @nox @emilio :)

nox · 2020-03-23T13:15:23Z

@bors-servo r+

Didn't realise that had not landed yet, sorry.

bors-servo · 2020-03-23T13:15:24Z

📌 Commit eea7eb4 has been approved by nox

bors-servo · 2020-03-23T13:22:41Z

⌛ Testing commit eea7eb4 with merge 718bebd...


        Auto merge of #25888 - ferjm:text.decoration.2020, r=nox

Partial text decoration support for layout 2020 - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #25166 - [X] There are tests for these changes

bors-servo · 2020-03-23T13:58:24Z

💔 Test failed - status-taskcluster


        Revert incorrect test result change.


        Add missing test result.


        Add missing test result.

jdm · 2020-03-23T15:18:13Z

@bors-servo r=nox

bors-servo · 2020-03-23T15:18:14Z

📌 Commit 132cee5 has been approved by nox

bors-servo · 2020-03-23T15:18:20Z

⌛ Testing commit 132cee5 with merge acd1467...

bors-servo · 2020-03-23T16:23:31Z

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

highfive assigned paulrouget Mar 4, 2020

highfive added the S-awaiting-review label Mar 4, 2020

ferjm unassigned paulrouget Mar 4, 2020

emilio reviewed Mar 4, 2020

View changes

highfive added the S-tests-failed label Mar 4, 2020

ferjm force-pushed the ferjm:text.decoration.2020 branch from f7fd0f9 to f57d4ee Mar 4, 2020

highfive removed the S-tests-failed label Mar 4, 2020

ferjm force-pushed the ferjm:text.decoration.2020 branch from d8210c4 to e95f1b5 Mar 5, 2020

ferjm changed the title ~~[WIP] Text decoration support for layout 2020~~ Partial text decoration support for layout 2020 Mar 5, 2020

highfive assigned emilio Mar 5, 2020

emilio requested changes Mar 5, 2020

View changes

ferjm added the A-layout/2020 label Mar 6, 2020

ferjm force-pushed the ferjm:text.decoration.2020 branch from e95f1b5 to d7cf405 Mar 9, 2020

ferjm added 4 commits Mar 4, 2020

Add support for text-decoration-style

689b413

Do not use buggy text_decorations_in_effect

83fc994

Propagate text decoration where needed

17948f3

Update test expectations with text-decoration support

Loading status checks…

eea7eb4

ferjm force-pushed the ferjm:text.decoration.2020 branch from ba033f4 to eea7eb4 Mar 23, 2020

highfive added S-awaiting-merge and removed S-awaiting-review labels Mar 23, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Mar 23, 2020

Revert incorrect test result change.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

1035d2f

highfive added S-awaiting-review and removed S-tests-failed labels Mar 23, 2020

jdm added 2 commits Mar 23, 2020

Add missing test result.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

b1a0c52

Add missing test result.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

132cee5

highfive added S-awaiting-merge and removed S-awaiting-review labels Mar 23, 2020

bors-servo merged commit acd1467 into servo:master Mar 23, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Mar 23, 2020

ferjm deleted the ferjm:text.decoration.2020 branch Apr 10, 2020

servo / servo

Partial text decoration support for layout 2020 #25888

Partial text decoration support for layout 2020 #25888

ferjm commented Mar 4, 2020

highfive commented Mar 4, 2020

ferjm commented Mar 4, 2020

bors-servo commented Mar 4, 2020

ferjm commented Mar 4, 2020

This comment has been minimized.

CYBAI commented Mar 4, 2020

bors-servo commented Mar 4, 2020

bors-servo commented Mar 4, 2020

CYBAI commented Mar 4, 2020

ferjm commented Mar 5, 2020

bors-servo commented Mar 5, 2020

bors-servo commented Mar 5, 2020

ferjm commented Mar 5, 2020

emilio left a comment

nox commented Mar 6, 2020

emilio commented Mar 6, 2020

nox commented Mar 10, 2020

ferjm commented Mar 23, 2020

nox commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

jdm commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

servo / servo

Join GitHub today

Partial text decoration support for layout 2020 #25888

Partial text decoration support for layout 2020 #25888

Conversation

ferjm commented Mar 4, 2020

highfive commented Mar 4, 2020

ferjm commented Mar 4, 2020

bors-servo commented Mar 4, 2020

ferjm commented Mar 4, 2020

This comment has been minimized.

emilio Mar 4, 2020

CYBAI commented Mar 4, 2020

bors-servo commented Mar 4, 2020

bors-servo commented Mar 4, 2020

CYBAI commented Mar 4, 2020

ferjm commented Mar 5, 2020

bors-servo commented Mar 5, 2020

bors-servo commented Mar 5, 2020

ferjm commented Mar 5, 2020

emilio left a comment

nox commented Mar 6, 2020

emilio commented Mar 6, 2020

nox commented Mar 10, 2020

ferjm commented Mar 23, 2020

nox commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

jdm commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020

bors-servo commented Mar 23, 2020