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

Basic handling framework for presentation attributes in Stylo, with handling for font-size and color #15331

Merged
merged 1 commit into from Feb 4, 2017

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 1, 2017

@highfive
Copy link

highfive commented Feb 1, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko_bindings/bindings.rs, components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/gecko/wrapper.rs, components/style/values/specified/length.rs
  • @KiChjang: components/script/dom/htmlfontelement.rs
  • @fitzgen: components/script/dom/htmlfontelement.rs
  • @emilio: components/style/gecko_bindings/bindings.rs, ports/geckolib/glue.rs, components/style/gecko_bindings/sugar/ns_css_value.rs, components/style/gecko/wrapper.rs, components/style/values/specified/length.rs

@highfive
Copy link

highfive commented Feb 1, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • 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 Feb 1, 2017
@Manishearth
Copy link
Member Author

@bors-servo are you still there?

@Manishearth
Copy link
Member Author

@bors-servo ping

@Manishearth
Copy link
Member Author

@bors-servo delegate=Manishearth

@bors-servo
Copy link
Contributor

✌️ @Manishearth can now approve this pull request

@Manishearth
Copy link
Member Author

@bors-servo are you still there?

1 similar comment
@Manishearth
Copy link
Member Author

@bors-servo are you still there?

@Manishearth
Copy link
Member Author

@bors-servo r=emilio,bz

@bors-servo
Copy link
Contributor

📌 Commit fdcb1fd has been approved by emilio,bz

@highfive highfive assigned emilio and unassigned jdm Feb 2, 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 Feb 2, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit fdcb1fd with merge ab78ac2...

bors-servo pushed a commit that referenced this pull request Feb 2, 2017
Basic handling framework for presentation attributes in Stylo, with handling for font-size and color

https://bugzilla.mozilla.org/show_bug.cgi?id=1330041

r=emilio,bz

<!-- 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/15331)
<!-- 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 Feb 2, 2017
@Manishearth
Copy link
Member Author

@bors-servo are you still there?

1 similar comment
@Manishearth
Copy link
Member Author

@bors-servo are you still there?

@bors-servo
Copy link
Contributor

🍰 There you are

@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 Feb 2, 2017
@Manishearth
Copy link
Member Author

@bors-servo r=emilio,bz

@bors-servo
Copy link
Contributor

📌 Commit 6e2cbe6 has been approved by emilio,bz

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Feb 4, 2017
@Manishearth
Copy link
Member Author

@bors-servo r=emilio,bz

@bors-servo
Copy link
Contributor

📌 Commit 718d6fe has been approved by emilio,bz

@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. S-needs-rebase There are merge conflict errors. labels Feb 4, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 718d6fe with merge de435f2...

bors-servo pushed a commit that referenced this pull request Feb 4, 2017
Basic handling framework for presentation attributes in Stylo, with handling for font-size and color

https://bugzilla.mozilla.org/show_bug.cgi?id=1330041

r=emilio,bz

<!-- 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/15331)
<!-- 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 Feb 4, 2017
…andling for font-size and color

This introduces a basic framework for servo's style system to be able
to query the style of presentation attributes which it can then insert
into the cascade. It uses that framework to implement the size and
color attributes on <font>.

There are a number of improvements that can be done on top of this:

 - Implement all other properties
 - Abstractify the ruledata parameter of the mappers using templates or virtual dispatch so that it can be a Servo decl block instead
 - Implement aforementiond abstraction over Servo decl blocks (this obsoletes the code in the first item above, so it might just be better to skip that and directly do this)
 - Replace uses of nsHTMLStyleSheet with an abstract base class containing common elements between Servo and Gecko

I'd prefer for these to be done in separate steps.
@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 Feb 4, 2017
@Manishearth
Copy link
Member Author

@bors-servo r=emilio,bz

ugh nocalclength broke everything :p

@bors-servo
Copy link
Contributor

📌 Commit 34ba00e has been approved by emilio,bz

@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 Feb 4, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 34ba00e with merge 57fb07e...

bors-servo pushed a commit that referenced this pull request Feb 4, 2017
Basic handling framework for presentation attributes in Stylo, with handling for font-size and color

https://bugzilla.mozilla.org/show_bug.cgi?id=1330041

r=emilio,bz

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

💔 Test failed - mac-rel-wpt1

@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 Feb 4, 2017
@Manishearth
Copy link
Member Author

Manishearth commented Feb 4, 2017

@bors-servo
Copy link
Contributor

⚡ Previous build results for android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev are reusable. Rebuilding only mac-rel-wpt1...

@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-gnu-dev, windows-msvc-dev
Approved by: emilio,bz
Pushing 57fb07e to master...

@bors-servo bors-servo merged commit 34ba00e into servo:master Feb 4, 2017
@Manishearth Manishearth deleted the stylo-presattr branch February 4, 2017 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants