Closed Bug 678173 Opened 8 years ago Closed 5 years ago

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


(Core :: CSS Parsing and Computation, enhancement)

Not set





(Reporter: karl, Unassigned, Mentored)




(Whiteboard: [good first bug][exterminationweek])


(1 file, 2 obsolete files)

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. 



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.
Component: General → Style System (CSS)
Product: Firefox → Core
QA Contact: general → style-system
Version: 6 Branch → Trunk
OS: Mac OS X → All
Hardware: x86 → All
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.
Ever confirmed: true
Severity: normal → enhancement
Summary: CSS View Mode - full screen not working on desktop → [view-mode] Implement the 'view-mode' media feature (CSS media queries)
Whiteboard: [good first bug][mentor=cpearce]
CSS media features:

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.
i would like to work on this bug for a school assignment however i need to get in contact with the developer.
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 as "roc" and "bz".  I'm usually around during work time US east coast time, while roc is on New Zealand time.
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.
Whiteboard: [good first bug][mentor=cpearce] → [good first bug][mentor=cpearce][exterminationweek]
Attached patch Patch (obsolete) — Splinter Review
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?
Attachment #8397660 - Flags: feedback?(cpearce)
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:

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 on attachment 8397660 [details] [diff] [review]

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...
Attachment #8397660 - Flags: feedback?(cpearce) → feedback+
Attached patch Patch v1 (obsolete) — Splinter Review
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:
Attachment #8397660 - Attachment is obsolete: true
Attachment #8398350 - Flags: review?(dbaron)
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...
Attachment #8398350 - Flags: review?(dbaron) → feedback?(dbaron)
Comment on attachment 8398350 [details] [diff] [review]
Patch v1

One comment from a quick skim:

>+    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;
Attachment #8398350 - Flags: feedback?(dbaron)
Wesj: are you willing to mentor this, if you're not working on it anymore?
Flags: needinfo?(wjohnston)
Attached patch WIP patchSplinter Review
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.
Attachment #8398350 - Attachment is obsolete: true
Flags: needinfo?(wjohnston)
Mentor: cpearce
Whiteboard: [good first bug][mentor=cpearce][exterminationweek] → [good first bug][exterminationweek]
This spec is deprecated (tho not officially). Web manifest defines "display modes", which are a better fit: 

Which is planned for here:
Closed: 5 years ago
Resolution: --- → WONTFIX
Removing the dev-doc-needed flag then. Re-add it if reopening the bug :-)
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.