Closed
Bug 1253078
Opened 7 years ago
Closed 7 years ago
Switch default environmental variable behavior for stylo builds
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(1 file)
798 bytes,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
Right now, when doing a stylo build, you also need to pass MOZ_STYLO=1 as an environmental variable to enable it. This is yet another thing that people need to remember to do, and is likely to be a source of confusion going forward. I do see some value in being able to dynamically turn off the runtime stylo behavior on an existing stylo build, but let's make that explicit, rather than the default.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8725919 -
Flags: review?(dholbert)
Comment 2•7 years ago
|
||
Comment on attachment 8725919 [details] [diff] [review] Switch MOZ_STYLO environmental variable to MOZ_DISABLE_STYLO. v1 Review of attachment 8725919 [details] [diff] [review]: ----------------------------------------------------------------- r=me; just one nit: ::: layout/base/nsPresContext.h @@ +1088,5 @@ > static bool StyloEnabled() > { > #ifdef MOZ_STYLO > + static bool disabled = PR_GetEnv("MOZ_DISABLE_STYLO"); > + return !disabled; Maybe worth adding a comment (outside the ifdef) to sum up the intended behavior here, in English, since the multiple variables + double negation of "!disabled" requires a little bit of staring to figure out. Something like: // Stylo is primarily enabled/disabled at build-time. (And in stylo-enabled // builds, we also allow it to be disabled at run-time via an env variable.)
Attachment #8725919 -
Flags: review?(dholbert) → review+
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1a1d1594492d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•