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

Allow all numeric values for SMIL values #16851

Merged
merged 4 commits into from May 14, 2017

Conversation

@hiikezoe
Copy link
Contributor

commented May 13, 2017

This is a PR for https://bugzilla.mozilla.org/show_bug.cgi?id=1357295 .

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented May 13, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/values/specified/length.rs, components/style/properties/properties.mako.rs, components/style/gecko/generated/bindings.rs, components/style/gecko/generated/structs_release.rs, components/style/parser.rs and 6 more
  • @KiChjang: components/script/dom/window.rs, components/script/dom/htmllinkelement.rs, components/script/dom/cssmediarule.rs, components/script/dom/cssstyledeclaration.rs, components/script/dom/htmlstyleelement.rs and 3 more
  • @fitzgen: components/script/dom/window.rs, components/script/dom/htmllinkelement.rs, components/script/dom/cssmediarule.rs, components/script/dom/cssstyledeclaration.rs, components/script/dom/htmlstyleelement.rs and 3 more
  • @emilio: ports/geckolib/glue.rs, components/style/values/specified/length.rs, components/style/properties/properties.mako.rs, components/style/gecko/generated/bindings.rs, components/style/gecko/generated/structs_release.rs and 7 more
@highfive

This comment has been minimized.

Copy link

commented May 13, 2017

warning Warning warning

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

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

Patches have been reviewed by @emilio .
I will land this tomorrow.

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2017

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

@hiikezoe hiikezoe force-pushed the hiikezoe:allow-all-numeric-values branch from ade07e7 to 5823401 May 13, 2017
@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

@bors-servo r=emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2017

📌 Commit 5823401 has been approved by emilio

@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

Yeah, I know I need to update bindings again after #16844 gets merged into autoland.

@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

Hiroyuki Ikezoe added 3 commits May 13, 2017
… PasingMode::AllowUnitlessLength.

We need another flag that represents allow-negative-number for SMIL, so
this enum will also comprise the another parsing mode that allows negative number.
assert_parsing_mode_match() is mostly the same as
assert_restyle_hints_match().
…values.

As per SVG spec [1], we should also parse negative color components values for
SMIL, but currently Gecko does not support it either.

[1] https://www.w3.org/TR/SVG/implnote.html#RangeClamping
@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

OK, git can merge this with #16844, but autoland unfortunately is closed...

@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

Now autoland is reopen, it's time to merge!

@bors-servo r=emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2017

📌 Commit 5823401 has been approved by emilio

Hiroyuki Ikezoe
@hiikezoe hiikezoe force-pushed the hiikezoe:allow-all-numeric-values branch from 5823401 to 48f6821 May 13, 2017
@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

@bors-servo r=emilio

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2017

📌 Commit 48f6821 has been approved by emilio

@hiikezoe

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2017

OK, homu, good boy. I don't know why updating this PR triggered #16849.

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2017

⌛️ Testing commit 48f6821 with merge 0900ad4...

bors-servo added a commit that referenced this pull request May 13, 2017
Allow all numeric values for SMIL values

<!-- Please describe your changes on the following line: -->
This is a PR for https://bugzilla.mozilla.org/show_bug.cgi?id=1357295 .
---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- 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/16851)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 14, 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 0900ad4 to master...

@bors-servo bors-servo merged commit 48f6821 into servo:master May 14, 2017
3 checks passed
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
@hiikezoe hiikezoe deleted the hiikezoe:allow-all-numeric-values branch May 14, 2017
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.