Closed Bug 1340696 Opened 5 years ago Closed 5 years ago

stylo: need to support system colors

Categories

(Core :: CSS Parsing and Computation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: bzbarsky, Assigned: manishearth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Testcase:

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

It sure would be nice if the border were visible, say.  ;)
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: nobody → manishearth
Status: NEW → ASSIGNED
Priority: -- → P1
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 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+
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
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/48f891a36a8f
stylo: Support system colors; r=heycam
https://hg.mozilla.org/mozilla-central/rev/48f891a36a8f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.