Last Comment Bug 678173 - [view-mode] Implement the 'view-mode' media feature (CSS media queries)
: [view-mode] Implement the 'view-mode' media feature (CSS media queries)
Status: RESOLVED WONTFIX
[good first bug][exterminationweek]
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
-- enhancement with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Jet Villegas (:jet)
Mentors: Chris Pearce (:cpearce)
http://www.w3.org/TR/view-mode/
Depends on:
Blocks: 545812
  Show dependency treegraph
 
Reported: 2011-08-11 06:36 PDT by Karl Dubost
Modified: 2014-11-29 09:45 PST (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (13.13 KB, patch)
2014-03-27 00:43 PDT, Wesley Johnston (:wesj)
cpearce: feedback+
Details | Diff | Splinter Review
Patch v1 (13.75 KB, patch)
2014-03-27 22:36 PDT, Wesley Johnston (:wesj)
no flags Details | Diff | Splinter Review
WIP patch (14.90 KB, patch)
2014-04-09 09:21 PDT, Wesley Johnston (:wesj)
no flags Details | Diff | Splinter Review

Description User image Karl Dubost 2011-08-11 06:36:10 PDT
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 User image Ioana (away) 2011-09-21 06:35:04 PDT
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 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-12 00:45:49 PDT
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 User image lmanuelab 2012-02-09 10:09:51 PST
i would like to work on this bug for a school assignment however i need to get in contact with the developer.
Comment 4 User image Boris Zbarsky [:bz] (still a bit busy) 2012-02-09 12:01:04 PST
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 User image Chris Pearce (:cpearce) 2012-02-10 02:11:20 PST
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 User image Wesley Johnston (:wesj) 2014-03-27 00:43:28 PDT
Created attachment 8397660 [details] [diff] [review]
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 User image Chris Pearce (:cpearce) 2014-03-27 14:18:27 PDT
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 User image Chris Pearce (:cpearce) 2014-03-27 14:20:59 PDT
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 User image Wesley Johnston (:wesj) 2014-03-27 22:36:14 PDT
Created attachment 8398350 [details] [diff] [review]
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 User image Wesley Johnston (:wesj) 2014-03-28 10:17:30 PDT
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 User image David Baron :dbaron: ⌚️UTC-8 2014-03-31 20:32:35 PDT
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 User image Mike Hoye [:mhoye] 2014-04-09 06:40:51 PDT
Wesj: are you willing to mentor this, if you're not working on it anymore?
Comment 13 User image Wesley Johnston (:wesj) 2014-04-09 09:21:12 PDT
Created attachment 8404066 [details] [diff] [review]
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 User image Marcos Caceres [:marcosc] 2014-11-26 14:11:17 PST
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 User image Jean-Yves Perrier [:teoli] 2014-11-29 09:45:48 PST
Removing the dev-doc-needed flag then. Re-add it if reopening the bug :-)

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