Closed Bug 132643 Opened 22 years ago Closed 22 years ago

Land the alerts infrastructure from ALERTS_SERVICE_BRANCH

Categories

(SeaMonkey :: UI Design, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: mscott, Assigned: mscott)

References

Details

Attachments

(2 files, 2 obsolete files)

This is a tracking bug I split off from 132401 to track the core alert service
which is getting added under xpfe\components\alerts.

I'm going to put the patches for the core alert service in this bug. 132401 will
have the mailnews specific changes for bringing up an animated alert for new
mail using this service. Please see 132401 or the mozilla landing page for more
information.
changing the QA contact to stephend since he owns #132401. This is an nsbeta1+ bug. 
Status: NEW → ASSIGNED
Keywords: nsbeta1+
QA Contact: paw → stephend
Target Milestone: --- → mozilla1.0
This patch contains the foundation for the animated alert service. 
It adds a new directory under xpfe\components:
xpfe\components\alerts

The alerts directory contains a small C++ class which implments
nsIAlertsService. This service just packages up the arguments for the alert and
kicks off the window open call. 

alert.xul and alert.js implement this chromeless, non focus stealing animated
window which slides up over the system tray. 

A new directory was created under:
themes\*skin name*\communicator\alerts
This contains alert.css which has the skinning rules for the alert.

Default prefs were added to all.js to allow some some customizations of the
alert dialog (such as how long the alert should stay open, how much animation
we want, etc).
Blocks: 132401
This supplemental patch is how I'm exposing the ability to create a native top
level window which has a widget type of popup. This is needed in order to
create the animated alert window such that it comes up on top of all windows
but never steals focus and never shows up as a window in the task tray.
A couple of nitpicks:

* In several places in alert.js, you are calling setTimeout like this:
setTimeout("fn();", 0);, but it's a little prettier to pass the function
directly like: setTimeout(fn, 0);

* In nsAlertsService::ShowAlertNotificatio, if you only need the ifptr
declaration inside the if (aAlertListener) block, why not just put it in there?

Comment on attachment 75460 [details] [diff] [review]
expose the ability to create a top level window with  window type: eWindowType_popup

  One step closer to the day when we run out of chrome flag bits! Woo hoo!
  Why not. Seems like something we should be able to do. However, I think you
should add this too, which will ignore the use of "popup" from untrusted script
("popup" seems a lot like "raised" to me):

--- embedding/components/windowwatcher/src/nsWindowWatcher.cpp	18 Mar 2002
23:33:45 -0000	    1.53
+++ embedding/components/windowwatcher/src/nsWindowWatcher.cpp	21 Mar 2002
23:17:40 -0000
@@ -1211,6 +1211,7 @@
     chromeFlags |= nsIWebBrowserChrome::CHROME_WINDOW_CLOSE;
     chromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_LOWERED;
     chromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_RAISED;
+    chromeFlags &= ~nsIWebBrowserChrome::CHROME_WINDOW_POPUP;
     //XXX Temporarily removing this check to allow modal dialogs to be
     //raised from script.  A more complete security based fix is needed.
     //chromeFlags &= ~nsIWebBrowserChrome::CHROME_MODAL;

(I hope that's readable; as I type it looks like the formatting will be screwed
up).
Attachment #75460 - Flags: review+
I incorporated Joe's comments into the core alert service implementation patch.
Thanks Joe.
Updated patch incorporating danm's security comment.
Attachment #75460 - Attachment is obsolete: true
Comment on attachment 75477 [details] [diff] [review]
expose the ability to create a top level window with window type: eWindowType_popup

transferring the r=danm
Attachment #75477 - Flags: review+
Incorporates Joe's comments.
Attachment #75449 - Attachment is obsolete: true
Comment on attachment 75477 [details] [diff] [review]
expose the ability to create a top level window with window type: eWindowType_popup

sr=hewitt
Attachment #75477 - Flags: superreview+
Comment on attachment 75480 [details] [diff] [review]
core alert service implementation

I got an sr=hewitt for this over aim after I incorporated his changes.
Attachment #75480 - Flags: superreview+
Comment on attachment 75480 [details] [diff] [review]
core alert service implementation

r=sspitzer

How hard would it be to make the alert show up on the bottom left, instead of
the bottom right?

you might get a request for that, from the i18n bidi people.  (the Hebrew
version of Windows is flipped, right?)
Attachment #75480 - Flags: review+
I've filed a couple follow up bugs to track some unfished issues: 
Bug #132731 --> make the animated alert window properly grow in width when given
really wide images (right now it expects the image to be <= 30 pixels or so).

Bug #132733 --> look into the internationalization issue about trying to put the
alert on the left hand side of the screen instead of the right hand side for
languages like Hebrew. (good catch Seth)
Comment on attachment 75480 [details] [diff] [review]
core alert service implementation

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75480 - Flags: approval+
Comment on attachment 75477 [details] [diff] [review]
expose the ability to create a top level window with window type: eWindowType_popup

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75477 - Flags: approval+
I just finished checking this in. I just flipped the switch so this is getting
built on windows (both nmake and gmake systems).

Marking fixed. Next up, landing the mailnews code which uses this service. 
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Using LXR and the nightly builds on various Windows OSs, I am able to verify
that the merge from the branch to the trunk was successful.

Verified FIXED
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.