1355003 - stylo: Implement shape-outside

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Shing Lyu [:shinglyu]]

Servo side issue:
https://github.com/servo/servo/issues/15958

Comment 1

[Shing Lyu [:shinglyu]]

Attachment: Failing test cases

Comment 2

[Bobby Holley (:bholley)]

Nazim, can you knock this one out? Should be pretty easy and blocks quite a number of tests.

Comment 3

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Sure!

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1355003 - stylo: Change some binding names to generalize them

Review commit: https://reviewboard.mozilla.org/r/129174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129174/

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1355003 - stylo: Update test expectations for shape-outside

Review commit: https://reviewboard.mozilla.org/r/129176/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129176/

Comment 6

[Manish Goregaokar [:manishearth]]

Comment on attachment 8857234 [details]
Bug 1355003 - stylo: Change some binding names to generalize them

https://reviewboard.mozilla.org/r/129174/#review131730

Comment 7

[Manish Goregaokar [:manishearth]]

Comment on attachment 8857235 [details]
Bug 1355003 - stylo: Update test expectations for shape-outside

https://reviewboard.mozilla.org/r/129176/#review131732

::: servo/components/style/gecko/conversions.rs:481
(Diff revision 1)
> +    }
> +
> +    // Will panic on NoBox or other GeometryBox items.
> +    // Ideally these would be implemented on Option<T>,
> +    // but coherence doesn't like that and TryFrom isn't stable
> +    impl From<StyleGeometryBox> for ShapeBox {

Can this instead be a function that returns Option? It's fine if you don't use From.

Comment 8

[Manish Goregaokar [:manishearth]]

You'll also need to update expectations. I did a try push (Automation > Trigger try on mozreview), https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c4046f50e7d

I have a script that can help update reftest expectations in bulk if you need it.

Comment 9

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Comment on attachment 8857234 [details]
Bug 1355003 - stylo: Change some binding names to generalize them

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129174/diff/1-2/

Comment 10

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Comment on attachment 8857235 [details]
Bug 1355003 - stylo: Update test expectations for shape-outside

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/129176/diff/1-2/

Comment 11

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Servo side: https://github.com/servo/servo/pull/16384

> Can this instead be a function that returns Option? It's fine if you don't use From.

Oh, I wrote this impl because it was implemented for `GeometryBox` but it looks like it's not needed for this change. I just removed it.
Also added a commit for test expectations.

Comment 12

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s aa95673a15d9 -d b1e0a9bc3655: rebasing 389545:aa95673a15d9 "Bug 1355003 - stylo: Change some binding names to generalize them r=manishearth"
merging layout/style/ServoBindings.cpp
merging layout/style/ServoBindings.h
rebasing 389546:6bb63b2cb4c6 "Bug 1355003 - stylo: Update test expectations for shape-outside r=manishearth" (tip)
merging layout/style/test/stylo-failures.md
warning: conflicts while merging layout/style/test/stylo-failures.md! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2eeb5637d6b2
stylo: Change some binding names to generalize them r=Manishearth
https://hg.mozilla.org/integration/autoland/rev/bfcf26eb6590
stylo: Update test expectations for shape-outside; r=Manishearth

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2eeb5637d6b2
https://hg.mozilla.org/mozilla-central/rev/bfcf26eb6590