Closed Bug 1355345 Opened 6 years ago Closed 6 years ago

stylo: Support font-display descriptor in @font-face rule

Categories

(Core :: CSS Parsing and Computation, enhancement, P2)

Product:
Core
Component:
CSS Parsing and Computation
Version:
53 Branch
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: xidorn, Assigned: ferjm)

References

Details

Attachments

(1 file)

Bug 1355345 - stylo: Support font-display descriptor in @font-face rule.
59 bytes, text/x-review-board-request
emilio
: review+
Details 
font-display is currently defined unofficially in https://tabatkins.github.io/specs/css-font-display/

Given bug 1157064 and bug 1157064, we probably want to add it for stylo.
Priority: -- → P2
Assignee: nobody → ferjmoreno
This is now specified in https://drafts.csswg.org/css-fonts-4/#font-display-desc, btw.

And while at it, drive-by reviewing: Fernando, define_css_keyword_enum! is your friend.
Comment on attachment 8868652 [details]
Bug 1355345 - stylo: Support font-display descriptor in @font-face rule.

https://reviewboard.mozilla.org/r/140236/#review143996

r=me with those two, thanks!

::: servo/components/style/font_face.rs:80
(Diff revision 1)
> +/// A font-display value for a @font-face rule.
> +/// The font-display descriptor determines how a font face is displayed based
> +/// on whether and when it is downloaded and ready to use.
> +#[cfg(feature = "gecko")]
> +#[derive(Clone, Debug, PartialEq, Eq)]
> +pub enum FontDisplay {

yes, you want to use `define_css_keyword_enum` for this, as nox said.

Also, any reason this _needs_ to be `cfg`'d, if it doesn't I'd rather leave it compiling for Servo too.

::: servo/components/style/gecko/rules.rs:125
(Diff revision 1)
> +        nscssvalue.set_enum(match self {
> +            FontDisplay::Auto => structs::NS_FONT_DISPLAY_AUTO as i32,
> +            FontDisplay::Block => structs::NS_FONT_DISPLAY_BLOCK as i32,
> +            FontDisplay::Swap => structs::NS_FONT_DISPLAY_SWAP as i32,
> +            FontDisplay::Fallback => structs::NS_FONT_DISPLAY_FALLBACK as i32,
> +            FontDisplay::Optional => structs::NS_FONT_DISPLAY_OPTIONAL as i32,

nit: You can put the cast after the `match`, I think.

match self {
    // ...
} as i32
Attachment #8868652 - Flags: review?(emilio+bugs) → review+
Pushed by ferjmoreno@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e486cf1ce1b0
stylo: Support font-display descriptor in @font-face rule. r=emilio
https://hg.mozilla.org/mozilla-central/rev/e486cf1ce1b0
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.