Land the alerts infrastructure from ALERTS_SERVICE_BRANCH

VERIFIED FIXED in mozilla1.0

Status

SeaMonkey
UI Design
VERIFIED FIXED
16 years ago
13 years ago

People

(Reporter: Scott MacGregor, Assigned: Scott MacGregor)

Tracking

Trunk
mozilla1.0
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

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

Comment 1

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

Comment 2

16 years ago
Created attachment 75449 [details] [diff] [review]
core alert service implementation

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

Updated

16 years ago
Blocks: 132401
(Assignee)

Comment 3

16 years ago
Created attachment 75460 [details] [diff] [review]
expose the ability to create a top level window with  window type: eWindowType_popup

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.

Comment 4

16 years ago
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 5

16 years ago
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+
(Assignee)

Comment 6

16 years ago
I incorporated Joe's comments into the core alert service implementation patch.
Thanks Joe.
(Assignee)

Comment 7

16 years ago
Created attachment 75477 [details] [diff] [review]
expose the ability to create a top level window with window type: eWindowType_popup

Updated patch incorporating danm's security comment.
Attachment #75460 - Attachment is obsolete: true
(Assignee)

Comment 8

16 years ago
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+
(Assignee)

Comment 9

16 years ago
Created attachment 75480 [details] [diff] [review]
core alert service implementation

Incorporates Joe's comments.
Attachment #75449 - Attachment is obsolete: true

Comment 10

16 years ago
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+
(Assignee)

Comment 11

16 years ago
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+
(Assignee)

Comment 13

16 years ago
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)
QA Contact: stephend → gchan

Comment 14

16 years ago
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 15

16 years ago
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+
(Assignee)

Comment 16

16 years ago
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
Last Resolved: 16 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.