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

Rewrite style images with a good dose of generics πŸ’‰ #16859

Merged
merged 9 commits into from May 15, 2017

Conversation

@nox
Copy link
Member

commented May 14, 2017

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented May 14, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs, components/style/properties/longhand/background.mako.rs, components/style/values/computed/length.rs, components/style/values/specified/mod.rs, components/style/values/computed/mod.rs and 8 more
  • @KiChjang: components/script/dom/element.rs
  • @fitzgen: components/script/dom/element.rs
  • @emilio: components/layout/display_list_builder.rs, components/style/properties/gecko.mako.rs, ports/geckolib/glue.rs, components/style/properties/longhand/background.mako.rs, components/style/values/computed/length.rs and 10 more
@highfive

This comment has been minimized.

Copy link

commented May 14, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@nox

This comment has been minimized.

Copy link
Member Author

commented May 14, 2017

@nox

This comment has been minimized.

Copy link
Member Author

commented May 14, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 14, 2017

βŒ›οΈ Trying commit 181f8f7 with merge 699bf1b368b82f3d4f4df5cc39a082fdb0e20189...

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

πŸ’” Test failed - linux-rel-wpt

@nox

This comment has been minimized.

Copy link
Member Author

commented May 15, 2017

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

βŒ›οΈ Trying commit 181f8f7 with merge 55f1fa1...

bors-servo added a commit that referenced this pull request May 15, 2017
Auto merge of #16859 - nox:gradients, r=<try>
Rewrite style images with a good dose of generics πŸ’‰

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16859)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

β˜€οΈ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
State: approved= try=True

@emilio
emilio approved these changes May 15, 2017
Copy link
Member

left a comment

Looks quite fancy! r=me with nits

};
for item in &self.items {
if !skip_comma {
try!(dest.write_str(", "));

This comment has been minimized.

Copy link
@emilio

emilio May 15, 2017

Member

nit: The rest of the function uses ?, perhaps this should too.

where G: HasViewportPercentage
{
fn has_viewport_percentage(&self) -> bool {
if let Image::Gradient(ref gradient) = *self {

This comment has been minimized.

Copy link
@emilio

emilio May 15, 2017

Member

nit: I think match is nicer than if let { ... } else { }, but feel free to disagree on this :)

nox added 9 commits May 11, 2017
Refactor Gradient
In a similar way to Position, now specified and computed gradients share
a common Gradient type defined in style::values::generics::image.

This allows us to reuse most code for many style traits like ToCss,
HasViewportPercentage and ToComputedValue.

The test changes are the fallout of the disappearance of AngleOrCorner::None,
which align our code to the spec for serialisation, where components that can
be omitted should be omitted.

@nox nox force-pushed the nox:gradients branch from 181f8f7 to dd38bba May 15, 2017

@nox

This comment has been minimized.

Copy link
Member Author

commented May 15, 2017

@bors-servo r=emilio p=2

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

πŸ“Œ Commit dd38bba has been approved by emilio

@highfive highfive assigned emilio and unassigned mbrubeck May 15, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

βŒ›οΈ Testing commit dd38bba with merge eb7314b...

bors-servo added a commit that referenced this pull request May 15, 2017
Auto merge of #16859 - nox:gradients, r=emilio
Rewrite style images with a good dose of generics πŸ’‰

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16859)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 15, 2017

β˜€οΈ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: emilio
Pushing eb7314b to master...

@bors-servo bors-servo merged commit dd38bba into servo:master May 15, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 16, 2017
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request May 16, 2017
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request May 17, 2017
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E

@nox nox deleted the nox:gradients branch May 17, 2017

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Sep 8, 2019
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E

UltraBlame original commit: e5ce4b08fb6f1238db4903f063e12abba75e81c3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Sep 8, 2019
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E

UltraBlame original commit: e5ce4b08fb6f1238db4903f063e12abba75e81c3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Sep 8, 2019
Bug 1341102: Update test expectations for servo/servo#16859. r=emilio
Patch by Anthony Ramine.

MozReview-Commit-ID: 7uTzBU8yG0E

UltraBlame original commit: e5ce4b08fb6f1238db4903f063e12abba75e81c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.