Closed
Bug 132643
Opened 22 years ago
Closed 22 years ago
Land the alerts infrastructure from ALERTS_SERVICE_BRANCH
Categories
(SeaMonkey :: UI Design, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.0
People
(Reporter: mscott, Assigned: mscott)
References
Details
Attachments
(2 files, 2 obsolete files)
3.02 KB,
patch
|
mscott
:
review+
hewitt
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
34.13 KB,
patch
|
sspitzer
:
review+
mscott
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
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•22 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•22 years ago
|
||
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 | ||
Comment 3•22 years ago
|
||
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•22 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 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•22 years ago
|
||
I incorporated Joe's comments into the core alert service implementation patch. Thanks Joe.
Assignee | ||
Comment 7•22 years ago
|
||
Updated patch incorporating danm's security comment.
Attachment #75460 -
Attachment is obsolete: true
Assignee | ||
Comment 8•22 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•22 years ago
|
||
Incorporates Joe's comments.
Attachment #75449 -
Attachment is obsolete: true
Comment 10•22 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•22 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 12•22 years ago
|
||
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•22 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•22 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•22 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•22 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
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
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•