Last Comment Bug 617052 - Use ITaskbarList2::MarkFullscreenWindow for proper windows taskbar integration
: Use ITaskbarList2::MarkFullscreenWindow for proper windows taskbar integration
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla9
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
: 660171 (view as bug list)
Depends on: 1191743 1210452
Blocks: 646374 680227
  Show dependency treegraph
 
Reported: 2010-12-06 10:21 PST by Jim Mathies [:jimm]
Modified: 2015-10-08 12:37 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for letting the taskbar know when we are going into full screen mode. (5.94 KB, patch)
2011-08-18 15:25 PDT, Brian R. Bondy [:bbondy]
no flags Details | Diff | Splinter Review
Patch for letting the taskbar know when we are going into full screen mode v2 (7.23 KB, patch)
2011-08-19 06:46 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Splinter Review
Formatting fix for previous push. (1.21 KB, patch)
2011-09-02 19:23 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Splinter Review

Description Jim Mathies [:jimm] 2010-12-06 10:21:51 PST
http://msdn.microsoft.com/en-us/library/bb774640(v=VS.85).aspx

We should support this via widget. It should help cleanup some of the full screen / taskbar z-order issues we have.
Comment 1 Brian R. Bondy [:bbondy] 2011-08-18 13:09:07 PDT
This does fix the problem with the auto hide taskbars by the way.
The thing is that we use ITaskbarList4 which requires Windows 7 but this function is available in XP which would be nice to have. 

I added Bug 680227.

I will use this bug as the autohide fix for Windows 7.
Comment 2 Brian R. Bondy [:bbondy] 2011-08-18 13:10:07 PDT
*** Bug 660171 has been marked as a duplicate of this bug. ***
Comment 3 Brian R. Bondy [:bbondy] 2011-08-18 15:25:13 PDT
Created attachment 554223 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode.

This fixes the auto hide taskbar problem.

I extended nsIWintaskbar for this call.

The extra version of prepareFullScreen with nsIDOMWindow was done because I think there are some tasks coming up that allows web content to control full screen in the "Web API".

Let me know if there is a way to get a nsIDOMWindow  from a nsWindow.
Comment 4 Jim Mathies [:jimm] 2011-08-18 18:44:11 PDT
Comment on attachment 554223 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode.

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

::: widget/public/nsIWinTaskbar.idl
@@ +194,5 @@
> +   *
> +   * @thow NS_ERROR_INVALID_ARG if the window is not a valid top level window
> +   * @thow NS_ERROR_UNEXPECTED for general failures.
> +   */
> +  void prepareFullScreenHWND(in voidPtr aWindow, in boolean aFullScreen);

We should mark this with [noscript] for now. (With ctypes we might actually be able to do this at some point but i don't believe we support native handles yet.) Besides I don't think we want to expose this to script until we have a good reason to do so.

Also, will we run into any problems on 64-bit builds passing a HANDLE through this interface?

::: widget/src/windows/WinTaskbar.cpp
@@ +452,5 @@
> +  
> +  return PrepareFullScreenHWND(toplevelHWND, aFullScreen);
> +}
> +
> +/* void prepareFullScreen(in nsIDOMWindow aWindow, in boolean aFullScreen); */

nit - comment copy paste error

::: widget/src/windows/nsWindow.cpp
@@ +2726,5 @@
>    }
>  
> +  if (!aFullScreen) {
> +    // Notify the taskbar that we have exited full screen mode.
> +    if (taskbarInfo) {

Might as well combine these together into a single if.
Comment 5 Brian R. Bondy [:bbondy] 2011-08-18 18:46:29 PDT
Thanks for the quick review :)

> Also, will we run into any problems on 64-bit builds passing a HANDLE through this interface?

No we wouldn't run into problems on x64 compiled apps because a HWND is a HANDLE which is a void* which would be 8 bytes on x64.
Comment 6 Jim Mathies [:jimm] 2011-08-18 18:54:46 PDT
Comment on attachment 554223 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode.

cool. r+ with nits addressed.
Comment 7 Brian R. Bondy [:bbondy] 2011-08-18 18:55:57 PDT
Found one more problem with this that I need to fix too if you start in full screen mode mTaskbar is not yet initialized. So I'll r? you again in any case.
Comment 8 Brian R. Bondy [:bbondy] 2011-08-19 06:46:51 PDT
Created attachment 554395 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode v2

Implemented review comments and fixed problem if you start in full screen.
Comment 9 Jim Mathies [:jimm] 2011-08-19 07:02:07 PDT
Comment on attachment 554395 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode v2

I took a second to spot that '@thow' vs. '@throw' change. :) Thanks for cleaning that up.
Comment 10 Brian R. Bondy [:bbondy] 2011-08-31 20:31:01 PDT
This was pushed to try, results:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=e8059c500d22
Comment 11 Brian R. Bondy [:bbondy] 2011-09-01 07:24:23 PDT
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/6faef8a5901c
Comment 12 Ed Morley [:emorley] 2011-09-01 13:53:55 PDT
http://hg.mozilla.org/mozilla-central/rev/6faef8a5901c
Comment 13 :Ms2ger 2011-09-02 01:54:36 PDT
Comment on attachment 554395 [details] [diff] [review]
Patch for letting the taskbar know when we are going into full screen mode v2

>+NS_IMETHODIMP
>+WinTaskbar::PrepareFullScreenHWND(void *aHWND, PRBool aFullScreen) {
>+  if (!Initialize())
>+    return NS_ERROR_NOT_AVAILABLE;
>+
>+    NS_ENSURE_ARG_POINTER(aHWND);
>+
>+    if (!::IsWindow((HWND)aHWND)) 
>+      return NS_ERROR_INVALID_ARG;
>+
>+    HRESULT hr = mTaskbar->MarkFullscreenWindow((HWND)aHWND, aFullScreen);
>+    if (FAILED(hr)) {
>+      return NS_ERROR_UNEXPECTED;
>+    }
>+
>+    return NS_OK;
>+}

Weird indentation here
Comment 14 Brian R. Bondy [:bbondy] 2011-09-02 19:23:21 PDT
Created attachment 558019 [details] [diff] [review]
Formatting fix for previous push.

No logic change.
Comment 15 Brian R. Bondy [:bbondy] 2011-09-16 15:48:07 PDT
http://hg.mozilla.org/mozilla-central/rev/fb5d85637013

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