The default bug view has changed. See this FAQ.

Use ITaskbarList2::MarkFullscreenWindow for proper windows taskbar integration

RESOLVED FIXED in mozilla9

Status

()

Core
Widget: Win32
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: jimm, Assigned: bbondy)

Tracking

(Depends on: 1 bug)

Trunk
mozilla9
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → netzen
(Assignee)

Updated

6 years ago
Depends on: 680227
(Assignee)

Comment 1

6 years ago
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.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 660171
(Assignee)

Comment 3

6 years ago
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.
Attachment #554223 - Flags: review?(jmathies)
(Assignee)

Updated

6 years ago
Blocks: 646374
(Reporter)

Comment 4

6 years ago
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.
(Assignee)

Comment 5

6 years ago
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.
(Reporter)

Comment 6

6 years ago
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.
Attachment #554223 - Flags: review?(jmathies) → review+
(Assignee)

Comment 7

6 years ago
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.
(Assignee)

Updated

6 years ago
Attachment #554223 - Flags: review+
(Assignee)

Comment 8

6 years ago
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.
Attachment #554223 - Attachment is obsolete: true
Attachment #554395 - Flags: review?(jmathies)
(Reporter)

Comment 9

6 years ago
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.
Attachment #554395 - Flags: review?(jmathies) → review+
(Assignee)

Updated

6 years ago
Blocks: 680227
No longer depends on: 680227
(Assignee)

Comment 10

6 years ago
This was pushed to try, results:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=e8059c500d22
(Assignee)

Comment 11

6 years ago
Pushed to mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/6faef8a5901c
http://hg.mozilla.org/mozilla-central/rev/6faef8a5901c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
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
(Assignee)

Comment 14

6 years ago
Created attachment 558019 [details] [diff] [review]
Formatting fix for previous push.

No logic change.
Attachment #558019 - Flags: review?(jmathies)
(Reporter)

Updated

6 years ago
Attachment #558019 - Flags: review?(jmathies) → review+
(Assignee)

Comment 15

6 years ago
http://hg.mozilla.org/mozilla-central/rev/fb5d85637013

Updated

2 years ago
Depends on: 1191743

Updated

2 years ago
Depends on: 1210452
You need to log in before you can comment on or make changes to this bug.