The default bug view has changed. See this FAQ.

stylo: need to support system colors

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
a month ago
6 days ago

People

(Reporter: bz, Assigned: manishearth)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 affected, firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Testcase:

  data:text/html,<textarea style="-moz-appearance: none">

It sure would be nice if the border were visible, say.  ;)
Blocks: 1324348
See also bug 1341697.
This involves adding system color support in rust-cssparser, as a new enum variant:

    pub enum Color {
        RGBA(RGBA),
        CurrentColor,
        System(SystemColor),  // new
    }

… with SystemColor a C-like enum.

In Servo, make them map to white or black as suggested in https://github.com/servo/rust-cssparser/issues/2.

In Stylo:

* Add bindings for the LookAndFeel::ColorID enum
* Have a Rust mapping of SystemColor to ColorID
* Add bindings for (a function wrapper for) the LookAndFeel::GetColor method, which takes ColorID and returns nscolor
* Use that nscolor for setting computed values.
(Assignee)

Updated

15 days ago
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 4

7 days ago
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9741971fa20565fb29426f434a6465f57ea4c9d8

Form widgets still won't work well because most form widget UA declarations include a :-moz-system-metric pseudo, which we don't parse, and we end up ignoring the entire selector list.


There also are system fonts in these declarations, which are ... weird.
Comment hidden (mozreview-request)

Comment 6

7 days ago
mozreview-review
Comment on attachment 8847825 [details]
Bug 1340696 - stylo: Support system colors;

https://reviewboard.mozilla.org/r/120742/#review122768

::: layout/style/ServoBindings.cpp:490
(Diff revision 2)
>  {
>    nsRuleNode::FillAllMaskLists(*aLayers, aMaxLen);
>  }
>  
> +nscolor Gecko_GetLookAndFeelSystemColor(int32_t aId,
> +                                        RawGeckoPresContextBorrowed aPresContext) {

Nit: { on new line.

::: layout/style/ServoBindings.cpp:493
(Diff revision 2)
>  
> +nscolor Gecko_GetLookAndFeelSystemColor(int32_t aId,
> +                                        RawGeckoPresContextBorrowed aPresContext) {
> +  bool useStandinsForNativeColors = aPresContext && !aPresContext->IsChrome();
> +  nscolor result;
> +  LookAndFeel::ColorID colorId = (LookAndFeel::ColorID) aId;

Nit: static_cast rather than C-style cast.

::: servo/components/style/properties/longhand/color.mako.rs:59
(Diff revision 2)
> +    % if product == "gecko":
> +        <%
> +            # These are actually parsed. See nsCSSProps::kColorKTable
> +            system_colors = """activeborder activecaption appworkspace background buttonface

File a Servo issue to support these values as described by Simon?

::: servo/components/style/properties/longhand/color.mako.rs:103
(Diff revision 2)
> +            fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
> +                let s = match *self {
> +                    % for color in system_colors + extra_colors:
> +                        LookAndFeel_ColorID::eColorID_${to_rust_ident(color)} => "${color}",
> +                    % endfor
> +                    LookAndFeel_ColorID::eColorID_LAST_COLOR => "",

Do we need to serialize this as the empty string, or can we unreachable!() it?
Attachment #8847825 - Flags: review?(cam) → review+
(Assignee)

Comment 7

7 days ago
Servo PR at https://github.com/servo/servo/pull/15974 . Feel free to autoland this once that lands if I'm not around (I should be).

Issue filed for pure servo impl at https://github.com/servo/servo/issues/15973
Comment hidden (mozreview-request)

Comment 9

7 days ago
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/48f891a36a8f
stylo: Support system colors; r=heycam

Comment 10

6 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/48f891a36a8f
Status: ASSIGNED → RESOLVED
Last Resolved: 6 days 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.