Open
Bug 1325472
Opened 7 years ago
Updated 2 years ago
./mach devtools-css-db produces different result depending on platform (adds/removes "grayscale" depending on whether it's being run on a mac)
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox53 affected)
NEW
Tracking | Status | |
---|---|---|
firefox53 | --- | affected |
People
(Reporter: MatsPalmgren_bugz, Unassigned)
Details
Attachments
(1 file)
917 bytes,
patch
|
Details | Diff | Splinter Review |
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)
Comment 1•7 years ago
|
||
(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)
Comment 2•7 years ago
|
||
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.
Comment 3•7 years ago
|
||
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)
Comment 4•7 years ago
|
||
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•6 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•