18518 - [FEATURE] nsWindow user notification

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[lchiang]

Need support for notification when a window has received something new

trudelle - this is the bug I filed based on
http://scopus/bugsplat/show_bug.cgi?id=361657 (internal bug system used before
bugzilla).  See comments there.

Comment 1

[Peter Trudelle]

Yes, this is needed for Messenger and other apps (see bugsplat equiv). assigning
to danm as p3 for m13, putting on dogfood radar.

Comment 2

[leger]

Putting on pdt+ radar.

Comment 3

[Jim Roskind]

Reformatted date field in whiteboard, without changing value (to help my query)

Comment 4

[Syd Logan]

Dan, this is what I think we should do: add to nsIWindow an interface called
FlashWindow. It needs two arguments, a rate (in milliseconds), and a command
(either stop or start). On Unix, and perhaps Mac, the rate is ignored. On Unix,
the only command implemented is start, and it simply raises the window to the
top (stop is a no-op). On Windows, win98 and NT2000 (if that ever ships), start
calls FlashWindowEx(), the dwFlags field of the FLASHWINFO struct is set to
FLASHW_TIMER, and the dwTimeout field is set to the specified interval. On
Windows95 and Windows NT 4, we set up an nsITimer that goes off the specified
interval and calls FlashWindow() until a stop command is received. The stop
command for windows 98 and NT 2000 (if that ever ships) simply calls
FlashWindowEx again, this time with the FLASHWINFO struct dwFlags field set to
FLASHW_STOP. On MacOS, we make use of the notification manager (described in
Inside Macintosh: Processes). I think for Mac we only make use of the
Notification Manager if mozilla is in the background. This is checked in the
FlashWindow interface when the start command is issued. Stopping occurs when
the application is brought to the foreground. I am not sure if it is proper to
raise the window if mozilla is in the foreground when the start command is
issued, or when the application is brought to the foreground again. Perhaps a
machead would be able to advise here. For IM, I gather we will be the ones that
call FlashWindow with the start command, and it is nsIWindow that will call it
with the stop command, once a "window raise" event or an "application is in the
foreground" event is received.

Comment 5

[don]

Re-assigned to Bill with a preliminary target completion date of 12/10.  Once
Bill gets a chance to look at this closer, he'll let us know whether he can
actually hit this date.

Comment 6

[Bill Law]

I'd like to explore two ideas that might simplify the implementation of this
feature:

1. Move the new "FlashWindow" method to a place not quite as low-level as
nsIWindow.  While Win32 appears to accept an arbitrary HWND as argument to
FlashWindow[Ex], I think this method is really only appropriate for top-level
windows.  I.e., we could move it to nsIWebShellWindow.  This would move it out
of the widget library and into the appshell lib (so other embedding apps
wouldn't have to pay a price for what they probably can't use).  Intuitively,
that seems more like the proper home for such a method.  I'm not sure how
difficult it would be to implement this method at that level.  We'd need access
to the lower-level widget (in order to trigger the notification) and to events
(to know when the window comes to the foreground to turn off the notification).

2. I think we could simplify the method by dropping the arguments and leaving
those up to the (per-platform) implementation.  Since other platforms are likely
not going to have FlashWindow[Ex] equivalents, it seems a bit presumptious to
design the method to match one platforms implementation detail.  I suspect that
each place we'd call FlashWindow would always pass the exact same arguments so
it seems we could simply move those values into the implementation (on the
platform where they were needed) rather than complicate the interface.
Basically, we'd just have a "flash" method that requests the window to "draw
attention to yourself in a manner appropriate for this platform, until the user
acknowledges you."  This would simplify greatly the "brain print" of this method
(it seems to me) and make implementations easier on non-Windows platforms.

Comments?

Comment 7

[don]

Confirming target completion date as 12/10.

Comment 8

[Bill Law]

I've implemented (in my tree) a new nsIWebShellWindow method and an
implementation for windows.  This works fine on NT4 (haven't tried Win98).  I
used the old-style FlashWindow regardless because it was a total PITA to try to
use FlashWindowEx.  First, that function isn't in my SDK headers.  Second, when
I declared it manually, I got a load-time error because the entry isn't in the
user32.dll (although it *is* in user32.lib so I could link; go figure).  Anyway,
the FlashWindow implementation is easy enough and should work regardless.

Here are the patches to add the method to nsIWebShellWindow.  You will also need
to add the file flashWin.cpp (below, also) which contains the Windows
implementation.

I've researched the Mac situation and I think we can start the notification no
problem.  I'm still a little unclear on how to terminate it after the user
brings the window to the foreground.  I'll be looking for Mac (and Unix)
assistance.

Index: public/nsIWebShellWindow.h
===================================================================
RCS file: /cvsroot/mozilla/xpfe/appshell/public/nsIWebShellWindow.h,v
retrieving revision 1.12
diff -w -r1.12 nsIWebShellWindow.h
44a45
>   NS_IMETHOD Flash() = 0;
Index: src/nsWebShellWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,v
retrieving revision 1.235
diff -w -r1.235 nsWebShellWindow.cpp
1815a1816,1826
> // Pull in appropriate implementation of Flash().
> #if defined( XP_PC )
>     #include "flashWin.cpp"
> // Add #elif defined( XP_platform ) and #include for platform here.
> #else
> NS_IMETHODIMP
> nsWebShellWindow::Flash() {
>     printf( "Flash not implemented on this platform yet!\n" );
>     return NS_OK;
> }
> #endif
2148a2160
>   if ( aShell ) {
2149a2162,2164
>   } else {
>       // ? Got here per bug #15209.  Probably shouldn't happen.
>   }
Index: src/nsWebShellWindow.h
===================================================================
RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsWebShellWindow.h,v
retrieving revision 1.82
diff -w -r1.82 nsWebShellWindow.h
128a129
>   NS_IMETHOD Flash();

Index: src/flashWin.cpp
===================================================================
/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
 *
 * The contents of this file are subject to the Netscape Public
 * License Version 1.1 (the "License"); you may not use this file
 * except in compliance with the License. You may obtain a copy of
 * the License at http://www.mozilla.org/NPL/
 *
 * Software distributed under the License is distributed on an "AS
 * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
 * implied. See the License for the specific language governing
 * rights and limitations under the License.
 *
 * The Original Code is Mozilla Communicator client code.
 *
 * The Initial Developer of the Original Code is Netscape Communications
 * Corporation.  Portions created by Netscape are
 * Copyright (C) 1998 Netscape Communications Corporation. All
 * Rights Reserved.
 *
 * Contributor(s):
 */
#include <windows.h>

// Pick some random timer ID.  Is there a better way?
#define NS_FLASH_TIMER_ID 0x011231984

// This function is called to do the flashing on older versions of Windows.
static VOID CALLBACK nsFlashTimerFunc( HWND hwnd, UINT uMsg, UINT idEvent, DWORD
dwTime ) {
    // Flash the window until we're in the foreground.
    if ( GetForegroundWindow() != hwnd ) {
        // We're not in the foreground yet.
        FlashWindow( hwnd, TRUE );
    } else {
        KillTimer( hwnd, idEvent );
    }
}

// Draw user's attention to this window until it comes to foreground.
NS_IMETHODIMP
nsWebShellWindow::Flash() {
    // Got window?
    if ( !mWindow ) {
        return NS_ERROR_NOT_INITIALIZED;
    }

    // Get window handle.
    HWND hwnd = (HWND)( mWindow->GetNativeData( NS_NATIVE_WINDOW ) );
    if ( !hwnd ) {
        return NS_ERROR_FAILURE;
    }
    // If window is in foreground, no notification is necessary.
    if ( GetForegroundWindow() != hwnd ) {
        // Kick off timer that does single flash till window comes to
foreground.
        SetTimer( hwnd, NS_FLASH_TIMER_ID, GetCaretBlinkTime(), nsFlashTimerFunc
);
    }

    return NS_OK;
}

Comment 9

[Syd Logan]

I'll be up 12/8, we can meet and talk about this?

Comment 10

[Bill Law]

Attachment: Diffs for second proposed fix

Comment 11

[Bill Law]

I attached diffs for another fix.  Up for review.

Here's a patch that enables this code when the browser completes a page load (it
makes a nice test).

Index: nsBrowserInstance.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/browser/src/nsBrowserInstance.cpp,v
retrieving revision 1.49
diff -w -r1.49 nsBrowserInstance.cpp
1552a1553,1563
> #ifdef DEBUG_law
>   // Flash the browser window when load completes (to test nsIWidget::Flash).
>   if ( mWebShellWin ) {
>     nsCOMPtr<nsIWidget> widget;
>     mWebShellWin->GetWidget( *getter_AddRefs( widget ) );
>     if ( widget ) {
>         widget->Flash();
>     }
>   }
> #endif
>

Comment 12

[Christopher Blizzard (:blizzard)]

I have a patch for gtk for this.  Let me know who can review it.

Comment 13

[Christopher Blizzard (:blizzard)]

Actually, I'll just stick it in here.  It's pretty small.  Someone gimme an r=
on it.

Index: nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/gtk/nsWindow.cpp,v
retrieving revision 1.213
diff -u -r1.213 nsWindow.cpp
--- nsWindow.cpp        1999/12/07 22:02:40     1.213
+++ nsWindow.cpp        1999/12/10 18:33:33
@@ -1738,6 +1738,20 @@
   return NS_OK;
 }

+NS_IMETHODIMP nsWindow::Flash(void)
+{
+  // get the next up moz area
+  GtkWidget *top_mozarea = GetMozArea();
+  if (top_mozarea) {
+    GtkWidget *top_window = gtk_widget_get_toplevel(top_mozarea);
+    if (top_window) {
+      // this will raise the toplevel window
+      gdk_window_show(top_window->window);
+    }
+  }
+  return NS_OK;
+}
+
 /* virtual */ void
 nsWindow::OnRealize(GtkWidget *aWidget)
 {
Index: nsWindow.h
===================================================================
RCS file: /cvsroot/mozilla/widget/src/gtk/nsWindow.h,v
retrieving revision 1.78
diff -u -r1.78 nsWindow.h
--- nsWindow.h  1999/12/06 16:23:34     1.78
+++ nsWindow.h  1999/12/10 18:33:33
@@ -81,6 +81,7 @@
   NS_IMETHOD           SetBackgroundColor(const nscolor &aColor);
   NS_IMETHOD           SetCursor(nsCursor aCursor);
   NS_IMETHOD           SetFocus(void);
+  NS_IMETHOD           Flash(void);

   void                 QueueDraw();
   void                 UnqueueDraw();

Comment 14

[Christopher Blizzard (:blizzard)]

Ok, I have an r= from pavlov.  I need to get permission to check it in though.

Comment 15

[Christopher Blizzard (:blizzard)]

Fix checked in for gtk.

Comment 16

[Alex Musil]

So the only platform left is the Mac?

Comment 17

[don]

And who is doing the Mac-specific work?  Shouldn't they own this bug now?

Comment 18

[Bill Law]

What Don says.  I'm clearing the [PDT+].  The fact that the feature isn't
operative yet only on the Mac may be enough to cause this to drop beneath the
[PDT+] radar.  We should also take this opportunity to pass it off to a Mac
engineer who is in a better position to implement nsWindow::Flash on the Mac.

To facilitate that, I'm reassigning this to the component owner.

Comment 19

[Syd Logan]

Time for me to update. As it turns out, we also need an interface in
nsIDOMWIndow and the soon to be released by travis web shell. I've worked all
the details out with vidur, and am about to checkin. This bug won't clear until
bugzilla 13374 has been fixed as a result.

Marked dependency on 13374. Once the mac implementation has been layed down,
*please* reassign this to me ([e-mail])

Comment 20

[Jim Roskind]

PDT says Mac functionality is important... so we really want this for dogfood.

Comment 21

[Peter Trudelle]

reassigning to sdagley as p1 for M12.  need estimated fix date.

Comment 22

[Steve Dagley]

As per syd's comments of 11/24 the Mac will use the notification manager to flash
the icon of the Mozilla process and play the user's alert sound.  This will only
occur if the app is in the background when Flash() is called and bringing the app
to the foreground will clear the notification.

Comment 23

[Syd Logan]

Perfect.

Comment 24

[chris hofmann]

is this really ready for checkin on monday 12/13?

Comment 25

[Steve Dagley]

Yes, it'll be ready for checkin on 12/13 (I'm implementing the Mac version now
and will get it reviewed in the AM)

Comment 26

[Syd Logan]

Note that the dependencies still need to be resolved before this feature can be
used -- waiting on a review from vidur on nsIDOMWindow changes, and travis
landing web shell changes.

Comment 27

[Steve Dagley]

Attachment: Implementation of Flash() method of nsWindow for Mac

Comment 28

[Steve Dagley]

The Mac changes to nsWindow to implement Flash() are ready checkin once I get a
review (the diffs for the Mac impl are attached if there's any volunteers)

Comment 29

[Alex Musil]

Mac impl looks fine to me.

Comment 30

[Steve Dagley]

Mac impl checked in, re-assigning to syd as requested

Comment 31

[Steve Dagley]

Houston, we have a problem.  Flash() is not a good name for this method. sfraser
has pointed out that nsWindow already has a Flash() method that takes a
nsPaintEvent and is used for the PaintFlashing debugging code.  We should use
some other name that doesn't collide with existing method names.  How about
GetUsersAttention()?  This also makes more sense for the Mac since we do not
actually flash the window.

Comment 32

[Christopher Blizzard (:blizzard)]

I like that name better.  It's more fitting.

Comment 33

[Alex Musil]

So we're waiting for web shell changes from Travis?

Who owns renaming the method?

Comment 34

[Alex Musil]

Clearing date and reassigning to Travis.

Travis - when do you plan to have your web shell change checked in?

Comment 35

[Syd Logan]

travis, back to me after you land the webshell changes i need, so i can make the
call to you.

Comment 36

[don]

Clearing PDT+ status and moving to M13.  C'mon, Travis isn't gonna get this done
until after the Webshell stuff lands and that isn't even gonna hold up dogfood.

Comment 37

[Alex Musil]

Agreed.  Should be PDT-.

Comment 38

[leger]

Putting on PDT- radar.

Comment 39

[don]

Replaced "DOGFOOD" with "FEATURE" in suammary.  Travis, I thought Bill already
implemented this?

Comment 40

[Syd Logan]

Don, travis is only part of this. Bill did the widget part, I did the
nsIDOMWindow part, and travis is the webshell part. Once travis has something
for me to call, it come back to me (it'll be reassigned to me) to add a call to
webshell, and do testing of everything.

Comment 41

[don]

Re-summarized.

Comment 42

[travis]

Moving to M14...

Comment 43

[travis]

Oh, yeah, I fixed this the other day.

Comment 44

[Claudius Gayle]

after giving up the better part of my morning to read all of this bug it would seem that after travis landed his work this bug is 
supposed to go back to syd for him to implement his part(and test it) so the whole thing can actually work. Then someone will tell 
me how to test it...
so I'm REOPENING in order to reassign it to syd. bugzilla spam to follow.

Comment 45

[Claudius Gayle]

back to syd

Comment 46

[Syd Logan]

This works fine for us in Commercial tree with IM. To test:

mac: it appears we need bndl resources added so the icon will flash in the 
toolbar. Otherwise, you should see evidence in the finder task menu when an IM 
is received and the conversation window is obscured.

windows: iconify the IM conversation window, send an IM, you should see the task 
tray item for the window flash

linux: either iconify the window, or give some other window focus, on receiving 
an IM, the window should grab focus and move to the top.

Comment 47

[lchiang]

claudius - my group can verify this.  Suzanne or Prass, can you verify?

Comment 48

[scalkins]

Win and Linux works as described.
On Mac what is meant by "mac: ... you should see evidence in the finder task 
menu when an IM is received and the conversation window is obscured."
Is the finder supposed to flash, or the finder menu item for Seamonkey AIM?
I can check this Monday. Whe I loked at it on Fridays build, there was no 
obvious flashing for minimized aim session windows.
 

Comment 49

[Claudius Gayle]

I imagine that part(Mac: ...) means that the seamonkey('Netscape') menu item in the finder task menu should at least have a 
bullet next to it indicating it wants your attention.

Comment 50

[Syd Logan]

Yes, the engineer who did the mac portion didn't completely finish the
implementation. We either need to pass an icon to the Mac toolbox or specify one
in BNDL resources, not sure which. I'm cc'ing this to smfr for help. I would
recommend verifying this bug since the big picture is done, and opening one for
Mac only, specifying that the job needs to be finished there. I'd make that bug
beta2.

I'm cc'ing simon in this, since he is my favorite mac-head.

Comment 51

[scalkins]

Verified for Mac and Linux 2000-02-14-09 M14 builds.
New Mac only bug will be opened for Mac issue.

Comment 52

[Claudius Gayle]

I assume you meant Win32 and Linux ;-). please note the new mac only bug number here in this bug so that those interested 
can still track the issue.

Comment 53

[scalkins]

Here it is!
The Mac only bug spun off this one is 
http://bugzilla.mozilla.org/show_bug.cgi?id=27744