1385469 - Stylo: Linear-gradient render different with and without stylo

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Fernando García Gómez, stripTM]

Attachment: background gradient test case

I suppose it is something induced and you want to stop supporting the old draft of css gradients. But I have seen web pages that are affected.

Using the current css engine with old css cross browser, the gradient goes in one direction and with Stylo is in the opposite direction.

Comment 1

[Cameron McCormack (:heycam)]

Here is another issue with -webkit-linear-gradient() directions.  It looks like a bare "bottom" or "top" is ignored, although "bottom right" or "top right" works.  Shing, since you worked on bug 1372821, do you want to take a look at this one?

Comment 2

[Cameron McCormack (:heycam)]

And thanks for the bug report, Fernando.

Comment 3

[Manish Goregaokar [:manishearth]]

This happens because -webkit-linear-gradient is defined to be an alias of linear-gradient, and this is using one of the many older deprecated syntaxes for it.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Manish Goregaokar [:manishearth] from comment #3)
> This happens because -webkit-linear-gradient is defined to be an alias of
> linear-gradient, and this is using one of the many older deprecated syntaxes
> for it.

That's... not because of that...

It is because gradient implementation is so complicated that you can never predict what would happen when you do an innocent-looking change.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Always set mBgPos for LineDirection::Vertical.

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

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1385469 - Add wpt for line direction in -webkit-prefixed linear gradient function.

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

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc3f1af1ef966f1545417c757d392d321858bf26

Comment 8

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

Comment on attachment 8893678 [details]
Always set mBgPos for LineDirection::Vertical.

https://reviewboard.mozilla.org/r/164784/#review170182

Looks good to me. Thanks for fixing!

Comment 9

[Manish Goregaokar [:manishearth]]

Oh, I thought it was because the syntax left out the `to`, which IIRC is invalid for linear gradient (and thus for -webkit-gradient *according to the spec* but we're not following that)

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

The fix itself is Servo-side only. I'm going to land that part now. The Gecko-side change is just adding new tests, so it is probably fine to land later.

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Servo PR: servo/servo#17971

Comment 12

[Daniel Holbert [:dholbert]]

Comment on attachment 8893679 [details]
Bug 1385469 - Add wpt for line direction in -webkit-prefixed linear gradient function.

https://reviewboard.mozilla.org/r/164786/#review170416

Comment 13

[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 065d6a5b2660 -d 4289bf021340: rebasing 411725:065d6a5b2660 "Always set mBgPos for LineDirection::Vertical. r=canaltinova"
note: rebase of 411725:065d6a5b2660 created no changes to commit
rebasing 411726:a1ba442dc72b "Bug 1385469 - Add wpt for line direction in -webkit-prefixed linear gradient function. r=dholbert" (tip)
merging testing/web-platform/meta/MANIFEST.json
warning: conflicts while merging testing/web-platform/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f914a210d73f
Add wpt for line direction in -webkit-prefixed linear gradient function. r=dholbert

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f914a210d73f

Comment 16

[Darkspirit]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> Servo PR: servo/servo#17971

Verified fixed in Nightly 57 x64 20170804193726 @ Debian Testing.