./mach devtools-css-db produces different result depending on platform (adds/removes "grayscale" depending on whether it's being run on a mac)

NEW
Unassigned

Status

Firefox Build System
General
a year ago
2 months ago

People

(Reporter: mats, Unassigned)

Tracking

Trunk
x86_64
Linux

Firefox Tracking Flags

(firefox53 affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
STR
1. in a clean mozilla-inbound tree, run "./mach devtools-css-db"

ACTUAL RESULTS
diff --git a/devtools/shared/css/generated/properties-db.js b/devtools/shared/css/generated/properties-db.js
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -3254,17 +3254,16 @@ exports.CSS_PROPERTIES = {
       "fixed",
       "flat",
       "flex",
       "flex-end",
       "flex-start",
       "forwards",
       "full-width",
       "geometricprecision",
-      "grayscale",
       "grid",
       "groove",
       "groupbox",
       "hanging",
       "hard-light",
       "hidden",
       "hide",
       "horizontal",

EXPECTED RESULTS
no change

Firefox Linux x86-64 debug build

It looks like 'grayscale' is still in use as a value for 'font-smoothing' and 'filter'?
I don't know, sometimes people forget to run this tool...  (e.g. bug 1321982)
(In reply to Mats Palmgren (:mats) from comment #0)
> It looks like 'grayscale' is still in use as a value for 'font-smoothing'
> and 'filter'?

Two updates/clarifications on this observation:
(1) RE "filter": In that CSS property, grayscale is a *function* rather than a keyword-value -- I don't think this part of the devtools DB is associated with that usage of "grayscale".  (But read on...)

(2) RE "font-smoothing": actually we don't implement "font-smoothing" but rather something called "-moz-osx-font-smoothing" (a mac-specific CSS property, enabled via a platform-dependent pref like so:
> #ifdef XP_MACOSX
> pref("layout.css.osx-font-smoothing.enabled", true);
> #else
> pref("layout.css.osx-font-smoothing.enabled", false);
> #endif
https://dxr.mozilla.org/mozilla-central/rev/855e6b2f6199189f37cea093cbdd1735e297e8aa/modules/libpref/init/all.js#2595-2601

Sample usage:
> -moz-osx-font-smoothing: grayscale;
https://dxr.mozilla.org/mozilla-central/rev/855e6b2f6199189f37cea093cbdd1735e297e8aa/layout/reftests/text/osx-font-smoothing-2-notref.html#13

SO: that explains why this keyword is in this file -- it must've been generated on a Mac -- and it makes sense why my Linux environment wants to remove it.
Summary: ./mach devtools-css-db produces wrong results (removing "grayscale" for no apparent reason) → ./mach devtools-css-db produces different result depending on platform (adds/removes "grayscale" depending on whether it's being run on a mac)
Created attachment 8836316 [details] [diff] [review]
hacky strawman patch (don't land)

Here's a patch that "fixes it" by removing the platform-dependence of this CSS property.

Specifically: if you apply this patch on Linux, and then run
 ./mach build && ./mach devtools-css-db
...then you won't see this issue anymore -- the ./mach devtools-css-db command won't remove "grayscale" like it otherwise would.

Clearly we don't want to take this exact patch. But I think this points us in a direction we could go -- just get this pref at a consistent setting regardless of platforms. Clearly ./mach devtools-css-db is running a some micro Firefox instance under the hood (since it seems to be reacting to default prefs), so we should probably just configure it to manually enable this about:config pref in its dummy Firefox profile before it does its work looking at the enabled properties/values.
Greg, maybe you'd be interested in taking this & giving comment 2 a shot?

(If you're on Mac, you can probably prototype a solution here by pretending you have the opposite problem -- i.e. you could remove "grayscale" from properties-db.js manually, and then try to fix things so that "./mach devtools-css-db" won't add it back automatically [via having it turn off the pref "layout.css.osx-font-smoothing.enabled" before it does any of its real work].  And then -- assuming we actually want to keep "grayscale" in this file -- the actual fix would be exactly what you come up with, but with the pref tweaked to true instead of false.)
Flags: needinfo?(gtatum)
I'm focused on a P0 project to improve some performance tooling in support of the Quantum Flow project, so I don't think I have capacity to work on this at the time, but I'd be happy to provide any reviews or answer any questions about how everything works. This database is a fallback that in most cases a live version is loaded over the remote debugging protocol. So if it's not 100% accurate for some system-dependent properties, there isn't a large amount of user-impact. That said it is really nice to have accurate devtools!
Flags: needinfo?(gtatum)

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.