Closed Bug 1462233 (css-env-1) Opened 6 years ago Closed 5 years ago

[META] Consider implementing CSS Environment Variables Module Level 1 ( env() )

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed

People

(Reporter: heycam, Assigned: emilio)

References

(Depends on 1 open bug, Blocks 1 open bug, )

Details

(4 keywords, Whiteboard: [geckoview])

Attachments

(3 files)

Blocks: 1421329
Summary: consider implementing env() → Consider implementing env() (CSS environment variables)
Upgrading this to a metabug. 

This is a meta-bug to track our support for css-env-1:
  https://drafts.csswg.org/css-env-1/ (editor's draft)

This bug will be marked as "Depends on" bugs that are needed to complete implementation of css-env-1.  The dependency tree can be used to view a list of these dependencies.
Alias: css-env-1
Keywords: meta
Summary: Consider implementing env() (CSS environment variables) → [META] Consider implementing env() (CSS environment variables)
Summary: [META] Consider implementing env() (CSS environment variables) → [META] Consider implementing CSS Environment Variables Module Level 1 ( env() )
Per https://github.com/w3c/csswg-drafts/issues/2888#issuecomment-402992901 if we implement env() we most likely must implement the safe-area-inset-* variables at the same time, since content is being written using those variables without fallback values.
Thought I'd put this up for feedback. Needs tests and such.
Depends on: 1501530
It's a bit useless to keep a set of invalid properties if we're going
to use them just to reject lookups into another key. This makes it more
consistent with the cascade / no-references code, and should not change
behavior.
Assignee: nobody → emilio
Actually, I probably should get somebody to commit to do the Fennec / GeckoView work...

Chris, James, do you know who that somebody could be? This is ready to land otherwise.

If somebody gets me the safe area insets into nsIDocument / nsPresContext that should be more than enough, should be similar with how viewport works. The insets do change when orientation changes and such though.
Flags: needinfo?(snorp)
Flags: needinfo?(cpeterson)
Emilio, what needs to be done on Android? Is the work the same for both Fennec and GeckoView?

Are you blocked from landing your core changes until the Android changes are ready?
Flags: needinfo?(cpeterson) → needinfo?(emilio)
(In reply to Chris Peterson [:cpeterson] from comment #6)
> Emilio, what needs to be done on Android?

We need to detect the display cutout in order to get the safe areas.

See https://webkit.org/blog/7929/designing-websites-for-iphone-x/ for fancy diagrams.

Blink has an Android implementation, see https://bugs.chromium.org/p/chromium/issues/detail?id=825890.

> Is the work the same for both Fennec and GeckoView?

I don't know, possibly?

> Are you blocked from landing your core changes until the Android changes are
> ready?

Not necessarily, but if it's not on the plans to do it, then I need to reconsider adding a pref, which in this case is unfortunately non-trivial.
Flags: needinfo?(emilio)
Adding [geckoview] whiteboard tag so the GeckoView team will triage this Android request
Whiteboard: [geckoview]
From https://developer.android.com/reference/android/view/WindowInsets#getDisplayCutout(), this API requires Android P SDK.  But our build environment supports Android O SDK (by bug 1352015), not P SDK,  We have to support Android P SDK to implement this on Android.
filed as bug 1503455 to support Android P SDK.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)
> (In reply to Chris Peterson [:cpeterson] from comment #6)
> > Emilio, what needs to be done on Android?
> 
> We need to detect the display cutout in order to get the safe areas.
> 
> See https://webkit.org/blog/7929/designing-websites-for-iphone-x/ for fancy
> diagrams.
> 
> Blink has an Android implementation, see
> https://bugs.chromium.org/p/chromium/issues/detail?id=825890.
> 
> > Is the work the same for both Fennec and GeckoView?
> 
> I don't know, possibly?
> 
> > Are you blocked from landing your core changes until the Android changes are
> > ready?
> 
> Not necessarily, but if it's not on the plans to do it, then I need to
> reconsider adding a pref, which in this case is unfortunately non-trivial.

Yeah, it doesn't look like it'll be difficult for us to provide the cutout info. Mainly blocked on the Android P SDK as Makoto said. I don't think you should block on us for this to land.
Flags: needinfo?(snorp)
Depends on: 1503656
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #11)
> Yeah, it doesn't look like it'll be difficult for us to provide the cutout
> info. Mainly blocked on the Android P SDK as Makoto said. I don't think you
> should block on us for this to land.

Emilio, I filed bug 1503656 for the follow-up work to support the Android cutout.

James says you can land without waiting for Android support, though I assume that means safe area feature should be pref'd off?

What is Fennec/GeckoView's current layout behavior on Android devices with cutouts? Like Safari's default layout avoiding the cutout in https://webkit.org/wp-content/uploads/default-inset-behavior.png? Or partially hidden by the cutout like https://webkit.org/wp-content/uploads/viewport-fit-cover.png?
Flags: needinfo?(emilio)
(In reply to Chris Peterson [:cpeterson] from comment #12)
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #11)
> > Yeah, it doesn't look like it'll be difficult for us to provide the cutout
> > info. Mainly blocked on the Android P SDK as Makoto said. I don't think you
> > should block on us for this to land.
> 
> Emilio, I filed bug 1503656 for the follow-up work to support the Android
> cutout.

Thanks!

> James says you can land without waiting for Android support, though I assume
> that means safe area feature should be pref'd off?

I think we should land this without a pref. https://bugzilla.mozilla.org/show_bug.cgi?id=1298537#c8 is an example of issue this causes unfortunately (for desktop as well).

> What is Fennec/GeckoView's current layout behavior on Android devices with
> cutouts? Like Safari's default layout avoiding the cutout in
> https://webkit.org/wp-content/uploads/default-inset-behavior.png? Or
> partially hidden by the cutout like
> https://webkit.org/wp-content/uploads/viewport-fit-cover.png?

In my particular device the OS doesn't let the cutout to overflow the application, but I'm not sure it applies to everything out there.
Flags: needinfo?(emilio)
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/15da260eaa3c
Implement the env() function with hardcoded zeros for safe-area-inset. r=heycam,firefox-style-system-reviewers
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/7945ede7e1b8
Simplify invalid custom property handling. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/15da260eaa3c
https://hg.mozilla.org/mozilla-central/rev/7945ede7e1b8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Note to MDN writers:

I've added a note to the Fx65 rel notes to cover this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#CSS

the compat data has been added (thanks ExE Boss!)

We have a page for the env() variable, but it needs some work to get it into shape, for example cssinfo macro adding, example adding, general checking to make sure it is up-to-date wirth the latest spec.

I've checked out the documentation on this, made sure the compat data is up to date, and added some more details and a simple example on the reference page:

https://developer.mozilla.org/en-US/docs/Web/CSS/env

Let me know if this looks OK. Thanks!

Also, a quick question — is there a way to define your own environment variables on a CSS document, or can you currently only use the user-agent-provided ones, i.e. the iOS values?

Flags: needinfo?(emilio)

Thanks! Looks good, yeah. I'd maybe use the fallback in the examples like:

  padding:
    env(safe-area-inset-top, 0px)
    env(safe-area-inset-right, 0px)
    env(safe-area-inset-bottom, 0px)
    env(safe-area-inset-left, 0px);

... Or maybe not, I don't know.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #19)

Also, a quick question — is there a way to define your own environment variables on a CSS document, or can you currently only use the user-agent-provided ones, i.e. the iOS values?

Well, you can use custom properties for that, that's what they're for.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)

Thanks! Looks good, yeah. I'd maybe use the fallback in the examples like:

  padding:
    env(safe-area-inset-top, 0px)
    env(safe-area-inset-right, 0px)
    env(safe-area-inset-bottom, 0px)
    env(safe-area-inset-left, 0px);

... Or maybe not, I don't know.

This is a good point; I added them.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #19)

Also, a quick question — is there a way to define your own environment variables on a CSS document, or can you currently only use the user-agent-provided ones, i.e. the iOS values?

Well, you can use custom properties for that, that's what they're for.

That's what I thought, but I played around with a few things and couldn't get it to work. What would work in this case?

I tried things like

:root { --bg-color: red }

p { color: env(--bg-color); }

var(--bg-color)?

Depends on: 1523935
You need to log in before you can comment on or make changes to this bug.