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

Implement parsing/serialization for -webkit-text-stroke property #13849

Closed
Manishearth opened this issue Oct 20, 2016 · 15 comments · Fixed by #14358
Closed

Implement parsing/serialization for -webkit-text-stroke property #13849

Manishearth opened this issue Oct 20, 2016 · 15 comments · Fixed by #14358
Labels
A-content/css Interacting with CSS from web content (parsing, serializing, introspection) A-stylo C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor.

Comments

@Manishearth
Copy link
Member

Manishearth commented Oct 20, 2016

https://developer.mozilla.org/en-US/docs/Web/CSS/-webkit-text-stroke

The grammar is <integer> <color>, which means that it must be an integer followed by a color.

See https://github.com/servo/servo/wiki/Property-hacking-guide for instructions on how to work on such bugs. This bug requires familiarity with Rust.

File: https://github.com/servo/servo/blob/master/components/style/properties/longhand/inherited_text.mako.rs

@Manishearth Manishearth added A-content/css Interacting with CSS from web content (parsing, serializing, introspection) E-less-complex Straightforward. Recommended for a new contributor. A-stylo labels Oct 20, 2016
@highfive
Copy link

Please make a comment here if you intend to work on this issue. Thank you!

@AgostonSzepessy
Copy link
Contributor

I'd like to try this one.

@Manishearth
Copy link
Member Author

Go ahead!

@Manishearth Manishearth added the C-assigned There is someone working on resolving the issue label Oct 21, 2016
@AgostonSzepessy
Copy link
Contributor

@Manishearth I have a a few questions about implementing <color>. For <rgb()> should I use cssparser::RGBA with A set to 1, and for <rgba()> should I use cssparser::RGBA? For <deprecated-system-color> should I create an enum with all the values listed on MDN? Also, is there a data structure for storing HSL and HSLA or do I need to create one?

@Manishearth
Copy link
Member Author

You don't need to implement color, it already exists as a type, like Length. See how backgroubd-color in background.mako.rs does it.

@AgostonSzepessy
Copy link
Contributor

@Manishearth I'd like to make sure that I'm implementing this correctly. I have

pub struct StrokeValues {
    pub length: Length,
    pub color: Color,
}

pub enum SpecifiedValue {
    Default,
    Specified(StrokeValues),
}

pub mod computed_value {
    use super::StrokeValues;
    pub struct T(Option<StrokeValues>);
}

In computed_value, should I be using a struct instead of a tuple struct, or is what I have correct?

@Manishearth
Copy link
Member Author

I'm sorry. I just realized that this is a shorthand, not a longhand, and we won't need this implemented till the longhands are. Feel free to pick up a different open bug.

@chenpighead
Copy link
Contributor

Hi @Manishearth, I'm wondering maybe we could implement both shorthand and longhand in this issue, if we do want this property eventually.

@Manishearth
Copy link
Member Author

Manishearth commented Nov 15, 2016

Yes, feel free to implement both!

@Manishearth Manishearth reopened this Nov 15, 2016
@chenpighead
Copy link
Contributor

Hi @AgostonSzepessy, are you still planning to work on this?

@AgostonSzepessy
Copy link
Contributor

@chenpighead No, you can take this issue if you want.

@chenpighead
Copy link
Contributor

Ok, thanks. I'll start to work on this next week.

@chenpighead
Copy link
Contributor

I've finished the parsing/serialization part. Will continue to work on gecko glue and feature verification on Stylo.

@wafflespeanut
Copy link
Contributor

Well, you can open a PR (with a note of WIP at the top) :)

@chenpighead
Copy link
Contributor

@wafflespeanut, thank you for the pointer. I'll use WIP note next time. :)

bors-servo pushed a commit that referenced this issue Nov 26, 2016
stylo - implement -webkit-text-fill-color and -webkit-text-stroke.

<!-- Please describe your changes on the following line: -->

Implement -webkit-text-fill-color property.
Implement -webkit-text-stroke property, along with -webkit-text-stroke-width and -webkit-text-stroke-color longhand properties.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13849 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/14358)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/css Interacting with CSS from web content (parsing, serializing, introspection) A-stylo C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants