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.

Sign up for GitHub

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

Jump to bottom

Stylo: @page support #16315

Merged
merged 4 commits into from
Apr 10, 2017
Merged

Stylo: @page support #16315

merged 4 commits into from
Apr 10, 2017

Conversation

jryans
Copy link
Sponsor Contributor

@jryans jryans commented Apr 9, 2017 •
edited by upsuper
Loading

Reviewed by upsuper in https://bugzilla.mozilla.org/show_bug.cgi?id=1345206.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors

This change is Reviewable

@highfive
Copy link

highfive commented Apr 9, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/bindings.rs, components/style/gecko_bindings/structs_debug.rs, components/style/gecko_bindings/structs_release.rs, components/style/supports.rs, components/style/gecko/arc_types.rs, components/style/stylesheets.rs, components/style/properties/longhand/margin.mako.rs, components/style/properties/declaration_block.rs, components/style/properties/data.py, components/style/properties/shorthand/margin.mako.rs, components/style/keyframes.rs, components/style/properties/properties.mako.rs
  • @KiChjang: components/script/dom/cssrule.rs
  • @fitzgen: components/script/dom/cssrule.rs
  • @emilio: components/style/gecko_bindings/bindings.rs, components/style/gecko_bindings/structs_debug.rs, ports/geckolib/glue.rs, components/style/gecko_bindings/structs_release.rs, components/style/supports.rs, components/style/gecko/arc_types.rs, components/style/stylesheets.rs, components/style/properties/longhand/margin.mako.rs, components/style/properties/declaration_block.rs, components/style/properties/data.py, components/style/properties/shorthand/margin.mako.rs, components/style/keyframes.rs, components/style/properties/properties.mako.rs

@highfive
Copy link

highfive commented Apr 9, 2017

warning Warning warning

  • These commits modify style and script code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 9, 2017
@upsuper
Copy link
Contributor

upsuper commented Apr 9, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit b4701ab has been approved by upsuper

@highfive highfive assigned upsuper and unassigned mbrubeck Apr 9, 2017
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 9, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit b4701ab with merge 797d366...

bors-servo pushed a commit that referenced this pull request Apr 9, 2017
Auto merge of #16315 - jryans:at-page-stylo, r=upsuper
797d366 
Stylo: @page support

Reviewed by upsuper in https://bugzilla.mozilla.org/show_bug.cgi?id=1345206.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- 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/16315)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - mac-dev-unit

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 9, 2017
@jryans
Copy link
Sponsor Contributor Author

jryans commented Apr 9, 2017

Ah, seems I forgot to check ./mach test-unit -p style for this part. 😓

@jryans
Servo parsing / serialization for @page
f0e849c 
Adds basic parsing and serialization for @page rules in Servo.  It is handled
in the same manner as a regular style rule.

MozReview-Commit-ID: JRr3DDGcUIl
@jryans
Ignore non-margin properties in @page rule
981571f 
Extend Servo's @page parsing to match Gecko's CSS 2.2 behavior, where only
margin properties are allowed in an @page rule.  Other properties are ignored.

MozReview-Commit-ID: IPYUlnkLYSb
@jryans
Servo glue for @page rule
a9ecebb 
Expose new glue functions and types on the Servo side for working with @page
rules from Gecko.

MozReview-Commit-ID: 5g13YldTr9
@jryans
Update Gecko bindings for @page support
579bc36
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 9, 2017
@jryans
Copy link
Sponsor Contributor Author

jryans commented Apr 9, 2017

Okay, I've fixed up the tests. The only added change passes the rule type during tests.

I also rebased and stripped "Bug X" and "r=" from commit messages, which I just learned is preferred.

r? @upsuper

@upsuper
Copy link
Contributor

upsuper commented Apr 9, 2017

@bors-servo delegate+

You can land whenever you are ready to land both this and the Gecko side change when this gets merged.

@bors-servo
Copy link
Contributor

✌️ @jryans can now approve this pull request

@jryans
Copy link
Sponsor Contributor Author

jryans commented Apr 10, 2017

@bors-servo r=xidorn p=10

@bors-servo
Copy link
Contributor

📌 Commit 579bc36 has been approved by xidorn

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 10, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 579bc36 with merge 80f6160...

bors-servo pushed a commit that referenced this pull request Apr 10, 2017
Auto merge of #16315 - jryans:at-page-stylo, r=xidorn
80f6160 
Stylo: @page support

Reviewed by upsuper in https://bugzilla.mozilla.org/show_bug.cgi?id=1345206.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- 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/16315)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ 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: xidorn
Pushing 80f6160 to master...

@bors-servo bors-servo merged commit 579bc36 into servo:master Apr 10, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 10, 2017
@bors-servo bors-servo mentioned this pull request Apr 10, 2017
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@upsuper upsuper

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jryans @highfive @upsuper @bors-servo @mbrubeck