678173 - [view-mode] Implement the 'view-mode' media feature (CSS media queries)

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, enhancement)

Description

[Karl Dubost]

User Agent: Opera/9.80 (Macintosh; Intel Mac OS X 10.6.8; U; fr) Presto/2.9.168 Version/11.50

Steps to reproduce:

I created a test for enabling a CSS in Full screen view mode only. 

Explanation
See http://my.opera.com/karlcow/blog/2011/08/10/full-screen-mode-and-css

Test
http://www.la-grange.net/2011/08/10/opera-test-fullscreen



Actual results:

When switching Firefox in full screen mode, the specific CSS instructions are not executed.


Expected results:

The CSS should be executed when switched to full-screen mode. 

Working in Opera 11.50 on macintosh, and maybe on Chromium linux.

Comment 1

[Ioana (away)]

Verified on:
Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110920 Firefox/9.0a1 (20110920085517)

This issue reproduces on the latest nightly (and also on the older Fx versions). It does work fine on Opera.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

CSS media features:
http://mxr.mozilla.org/mozilla-central/find?string=nsmediafeatures

You'd want to get an nsIWidget from the document's nsIPresShell (probably via FrameManager()->GetRootFrame()->GetWindow()) and then call some methods on it (probably GetSizeMode at least).

Also need to respond to NS_SIZEMODE_EVENT in PresShell::HandleEvent to dynamically update styles.

Comment 3

[lmanuelab]

i would like to work on this bug for a school assignment however i need to get in contact with the developer.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Chris Pearce is on leave right now, but either I or Robert O'Callahan can probably help you out as needed.  His address is attached to comment 2; mine is attached to this comment.  You can also find us on IRC in the #developers channel on irc.mozilla.org as "roc" and "bz".  I'm usually around during work time US east coast time, while roc is on New Zealand time.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

lmanuelab: You can take a look at my patch for the -moz-full-screen media query (bug 545812, attachment 552297 [details] [diff] [review]) to give you an idea how to implement this. Note that patch never landed, and the -moz-full-screen media query had a similar spec to this one, but we decided to implement the view-mode CSS media queries here instead. You can basically copy that patch, but update change it to reflect the view-mode spec instead.

As Roc says, you'll need to call PostMediaFeatureValuesChangedEvent() (on the PresShell's  PresContext, similar to how I do in attachment 552297 [details] [diff] [review]) when NS_SIZEMODE_EVENT is handled in PresShell::HandleEvent to update the queries' state.

You'll still need to have a similar function as GetFullScreen() (from attachment 552297 [details] [diff] [review]) but it will need to reflect the view-mode query values instead of course.

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

I was curious today if we supported this and it seemed simple to implement. This works for me on a linux box. The tests aren't done.

You have good advice on how to minimize/maximize/fullscreen a window in a test? I see some browser-chrome tests do it and tried to expose that in SpecialPowers, but I've never edited it and wondered if there were betters ways.

Also, should unfocused tabs report that they're minimized?

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Adding window minimize/maximize to SpecialPowers sounds fine to me, but you should ask the people who normally review it, rather than taking my word on it. ;)

W.R.T. testing, see the fullscreen test and supporting files:
http://mxr.mozilla.org/mozilla-central/source/content/html/content/test/test_fullscreen-api.html?force=1

Be aware though, that you have to be very, very careful that the fullscreen transition has finished before you test state, as the "fullscreenchange" events can fire before the transition is complete and the window is at a stable size. This is a quirk of our implementation, and it's hard to fix across all platforms, so I never found the time to fix this. The fullscreen tests have some logic in them to wait until the window has reached the target size. You've also got to be careful about test behaviour when run on systems with multiple monitors, and the monitors in varied logical positions.

I don't think that tabs that are in the background (or unfocused but still visible on another monitor for that matter) should be considered minimized, since they won't be reflowed when they transition to this state.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8397660 [details] [diff] [review]
Patch

Review of attachment 8397660 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, a layout peer should review this, someone like bz or dbaron, or someone they suggest.

::: testing/specialpowers/content/specialpowersAPI.js
@@ +1443,5 @@
>    },
>  
> +  _waitForActive: function(window) {
> +    return new window.Promise((function(resolve, reject) {
> +      window.setTimeout(resolve, 1000);

Using setTimeout in tests is a bad, as it can cause orange if the machine is under load...

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Patch v1

These tests pass well for me locally now. I wrote a chrome-mochitest to make it easier to minimize/maximize the window. Pushed to try, but I figured I'd ask for review in parallel:

https://tbpl.mozilla.org/?tree=Try&rev=8ac19a31e778

Comment 10

[Wesley Johnston (:wesj)]

Comment on attachment 8398350 [details] [diff] [review]
Patch v1

This didn't make try happy. Moving to f? while I investigate. I suspect animations or async behavior are hurting things...

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8398350 [details] [diff] [review]
Patch v1

One comment from a quick skim:

>+    PRInt32 viewMode = NS_STYLE_VIEWMODE_WINDOWED;
>+
>+    nsCOMPtr<nsIWidget> widget = aPresContext->GetRootWidget();
>+    int32_t mode = widget ? widget->SizeMode() : 0;

0 doesn't seem to make much sense as a default here; it's not clear which enum value 0 corresponds to.  Furthermore, maybe it makes more sense to leave (or set) aResult as eCSSUnit_Null -- will that produce some reasonable "nothing"-ish behavior?  (I didn't check.)

>+    switch (mode) {
>+        case nsSizeMode_Minimized:
>+            viewMode = NS_STYLE_VIEWMODE_MINIMIZED;
>+            break;
>+        case nsSizeMode_Maximized:
>+            viewMode = NS_STYLE_VIEWMODE_MAXIMIZED;
>+            break;
>+        case nsSizeMode_Fullscreen:
>+            viewMode = NS_STYLE_VIEWMODE_FULLSCREEN;
>+            break;
>+        default:
>+            NS_WARNING("Illegal window state for this window");

Does this really not happen?  Seems like there are likely other states.

>+            break;
>+        }
>+
>+    aResult.SetIntValue(viewMode, eCSSUnit_Enumerated);
>+    return NS_OK;

Comment 12

[Mike Hoye [:mhoye]]

Wesj: are you willing to mentor this, if you're not working on it anymore?

Comment 13

[Wesley Johnston (:wesj)]

Attachment: WIP patch

I'm probably not the best, but I can give some pointers if someone wants. I haven't totally stopped on this, but its way at the bottom of things I'm working on, so if someone else has cycles, feel free to steal. This was my latest WIP. I'm not sure if it builds or not...

I was mostly struggling with getting some reliable tests. Minimizing/maximizing windows doesn't seem to make the tests happy. The ones in the original patch passed on some platforms, and failed on others. There are some other bugs here as well though. For instance, I made inactive windows return "minimized", but I don't think the media state is actually updated when windows are marked inactive yet.

Comment 14

[Marcos Caceres [:marcosc]]

This spec is deprecated (tho not officially). Web manifest defines "display modes", which are a better fit:

http://w3c.github.io/manifest/#display-modes 

Which is planned for here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1104916

Comment 15

[Jean-Yves Perrier [:teoli]]

Removing the dev-doc-needed flag then. Re-add it if reopening the bug :-)