Closed Bug 18518 Opened 20 years ago Closed 20 years ago

[FEATURE] nsWindow user notification

Categories

(Core :: XUL, defect, P1, major)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: lchiang, Assigned: slogan)

References

Details

Attachments

(2 files)

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.
Assignee: trudelle → danm
Whiteboard: [Dogfood]
Target Milestone: M12
Yes, this is needed for Messenger and other apps (see bugsplat equiv). assigning
to danm as p3 for m13, putting on dogfood radar.
Summary: Need support for notification when a window has received something new → [DOGFOOD] Need support for notification when a window has received something new
Whiteboard: [Dogfood] → [PDT+]
Putting on pdt+ radar.
Whiteboard: [PDT+] → [PDT+] (unscheduled: a description of the problem would be helpful)
Whiteboard: [PDT+] (unscheduled: a description of the problem would be helpful) → [PDT+] sched: 5 Dec, let's say
Target Milestone: M12 → M13
Whiteboard: [PDT+] sched: 5 Dec, let's say → [PDT+] sched: 12/5, let's say
Reformatted date field in whiteboard, without changing value (to help my query)
Whiteboard: [PDT+] sched: 12/5, let's say → [PDT+] sched: 12/5
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.
Assignee: danm → law
Whiteboard: [PDT+] sched: 12/5 → [PDT+] 12/10??? completion
Target Milestone: M13 → M12
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.
Status: NEW → ASSIGNED
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?
Whiteboard: [PDT+] 12/10??? completion → [PDT+] 12/10 completion
Confirming target completion date as 12/10.
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;
}
Blocks: 20870
Priority: P3 → P1
I'll be up 12/8, we can meet and talk about this?
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
>
I have a patch for gtk for this.  Let me know who can review it.
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();
Ok, I have an r= from pavlov.  I need to get permission to check it in though.
Fix checked in for gtk.
So the only platform left is the Mac?
And who is doing the Mac-specific work?  Shouldn't they own this bug now?
Assignee: law → trudelle
Status: ASSIGNED → NEW
Whiteboard: [PDT+] 12/10 completion
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.
Depends on: 13374
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 (syd@netscape.com)
Whiteboard: [PDT+]
PDT says Mac functionality is important... so we really want this for dogfood.
Assignee: trudelle → sdagley
Summary: [DOGFOOD] Need support for notification when a window has received something new → [DOGFOOD] Need notification when a window has received something new
Whiteboard: [PDT+] → [PDT+] need estimated fix date
reassigning to sdagley as p1 for M12.  need estimated fix date.
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.
Status: NEW → ASSIGNED
Whiteboard: [PDT+] need estimated fix date → [PDT+] 12/13
Perfect.
Blocks: 21564
is this really ready for checkin on monday 12/13?
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)
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.
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)
Mac impl looks fine to me.
Assignee: sdagley → syd
Status: ASSIGNED → NEW
Mac impl checked in, re-assigning to syd as requested
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.
I like that name better.  It's more fitting.
So we're waiting for web shell changes from Travis?

Who owns renaming the method?
Assignee: syd → travis
Whiteboard: [PDT+] 12/13 → [PDT+]
Clearing date and reassigning to Travis.

Travis - when do you plan to have your web shell change checked in?
travis, back to me after you land the webshell changes i need, so i can make the
call to you.
Whiteboard: [PDT+]
Target Milestone: M12 → M13
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.
Agreed.  Should be PDT-.
Whiteboard: [PDT-]
Putting on PDT- radar.
Blocks: 22176
Summary: [DOGFOOD] Need notification when a window has received something new → [FEATURE] Need notification when a window has received something new
Whiteboard: [PDT-]
Replaced "DOGFOOD" with "FEATURE" in suammary.  Travis, I thought Bill already
implemented this?
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.
Summary: [FEATURE] Need notification when a window has received something new → [FEATURE] nsWindow user notification
Re-summarized.
Status: NEW → ASSIGNED
Target Milestone: M13 → M14
Moving to M14...
Oh, yeah, I fixed this the other day.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
back to syd
Assignee: travis → syd
Status: REOPENED → NEW
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.
Status: NEW → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
claudius - my group can verify this.  Suzanne or Prass, can you verify?
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.
 
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.
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.
Verified for Mac and Linux 2000-02-14-09 M14 builds.
New Mac only bug will be opened for Mac issue.
Status: RESOLVED → VERIFIED
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.
Here it is!
The Mac only bug spun off this one is 
http://bugzilla.mozilla.org/show_bug.cgi?id=27744
No longer blocks: 20870
No longer blocks: 21564
No longer blocks: 22176
You need to log in before you can comment on or make changes to this bug.