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

stylo: support font-variant shorthand #17310

Merged
merged 2 commits into from Jun 14, 2017

Conversation

@chenpighead
Copy link
Contributor

chenpighead commented Jun 14, 2017

From gecko bug: Bug 1356134


This change is Reviewable

Avoid early returning Err() in parsers, so we could let the the caller
of the parsers to handle the rest of input, and return error if it requires
parsing entirely.

The point is let returning Err() stay inside input.try(), so we can count
on input.try() to restore the position when parsing invalid idents.

From gecko bug: Bug 1356134 (https://bugzilla.mozilla.org/show_bug.cgi?id=1356134)
@highfive
Copy link

highfive commented Jun 14, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/font.mako.rs, components/style/properties/shorthand/font.mako.rs
  • @emilio: components/style/properties/longhand/font.mako.rs, components/style/properties/shorthand/font.mako.rs
@highfive
Copy link

highfive commented Jun 14, 2017

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@chenpighead
Copy link
Contributor Author

chenpighead commented Jun 14, 2017

@bors-servo r=xidorn

@bors-servo
Copy link
Contributor

bors-servo commented Jun 14, 2017

@chenpighead: 🔑 Insufficient privileges: Not in reviewers

@upsuper
Copy link
Member

upsuper commented Jun 14, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jun 14, 2017

📌 Commit 6b587e5 has been approved by upsuper

@chenpighead
Copy link
Contributor Author

chenpighead commented Jun 14, 2017

No idea why I still can't talk to @bors-servo.... :/

@chenpighead chenpighead force-pushed the chenpighead:stylo-font-variant branch from 6b587e5 to d55af14 Jun 14, 2017
We still count on Bug 1356124 to fix font-variant-alternates longhand, so we
can get font-variant shorthand work properly.

Some comments about Bug 1356124 have been removed, since this shorthand code
should just work once we fix Bug 1356124.

From gecko bug: Bug 1356134 (https://bugzilla.mozilla.org/show_bug.cgi?id=1356134)
@chenpighead chenpighead force-pushed the chenpighead:stylo-font-variant branch from d55af14 to c7c7538 Jun 14, 2017
@upsuper
Copy link
Member

upsuper commented Jun 14, 2017

@bors-servo delegate+

@bors-servo
Copy link
Contributor

bors-servo commented Jun 14, 2017

✌️ @chenpighead can now approve this pull request

@chenpighead
Copy link
Contributor Author

chenpighead commented Jun 14, 2017

@bors-servo r=xidorn

@bors-servo
Copy link
Contributor

bors-servo commented Jun 14, 2017

📌 Commit c7c7538 has been approved by xidorn

@bors-servo
Copy link
Contributor

bors-servo commented Jun 14, 2017

Testing commit c7c7538 with merge c6da6ba...

bors-servo added a commit that referenced this pull request Jun 14, 2017
stylo: support font-variant shorthand

From gecko bug: [Bug 1356134](https://bugzilla.mozilla.org/show_bug.cgi?id=1356134)

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

bors-servo commented Jun 14, 2017

@bors-servo bors-servo merged commit c7c7538 into servo:master Jun 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.