User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082003 SeaMonkey/2.0a1pre When bug #270443 has been fixed, SeaMonkey can use the notification bar to notify users when an XP Install has been blocked. Reproducible: Always Steps to Reproduce: 1. 2. 3.

This adds a special notificationbox binding which handles notifications for blocked XPInstalls.

Comment on attachment 287565 [details] [diff] [review] patch I wasn't able to trigger a notification when xpinstall was disabled. Bug? >+.browser-notificationbox { >+ -moz-binding: url("chrome://navigator/content/browserNotification.xml#browser-notificationbox"); >+} By comparison with our other toolkit overrides this should be chrome://communicator/content/bindings/notification.xml >+ <field name="_stringBundleURI" readonly="true"> >+ "chrome://navigator/locale/browserNotification.properties" >+ </field> >+ >+ <field name="_brandStringBundleURI" readonly="true"> >+ "chrome://branding/locale/brand.properties" >+ </field> Because of a recent crash fix, fields now only evaluate on first use. This means that you can inline the code to get the string bundle into the field. If you don't like that idea, then I'd prefer separate properties for the two bundles, rather than the method and fields approach. >+ var brandStringBundle = this.getStringBundle(this._brandStringBundleURI); >+ var brandShortName = brandStringBundle.GetStringFromName("brandShortName"); You only use this once, no point getting it in the other cases. >+ callback: function() { Interestingly you didn't name this callback ;-) >+ <destructor> >+ <![CDATA[ >+ var os = Components.classes["@mozilla.org/observer-service;1"] >+ .getService(Components.interfaces.nsIObserverService); >+ os.removeObserver(this, "xpinstall-install-blocked"); >+ ]]> >+ </destructor> I'm not sure you can actually rely on the destructor running; removeTab has to call destroy().

Addresses nits. I'm actually not sure if _stringBundle and _brandStringBundle in notification.xml should be cleared also on destroy().

Comment on attachment 287771 [details] [diff] [review] patch (v2) Sorry for the delay. >+xpinstallPromptAllowButton=Allow I've decided that Allow is too confusing. Please use use this instead: xpinstallPromptInstallButton=Install...

Replace "Allow" with "Install..."

Comment on attachment 288202 [details] [diff] [review] patch (for check in) checked in by request of twanno via IRC

Comment on attachment 288202 [details] [diff] [review] patch (for check in) >Index: suite/browser/navigator.css >=================================================================== >RCS file: /cvsroot/mozilla/suite/browser/navigator.css,v >retrieving revision 1.15 >diff -u -8 -p -r1.15 navigator.css >--- suite/browser/navigator.css 29 Jul 2007 23:15:31 -0000 1.15 >+++ suite/browser/navigator.css 10 Nov 2007 23:35:30 -0000 >@@ -17,16 +17,22 @@ tabbrowser { > .tabbrowser-tabs > .tabbrowser-tab { > -moz-binding: url("chrome://navigator/content/tabbrowser.xml#tabbrowser-tab"); > } > > .tabs-closebutton-box > .tabs-closebutton { > -moz-binding: url("chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton"); > } > >+/* ::::: notification box ::::: */ >+ >+.browser-notificationbox { >+ -moz-binding: url("chrome://communicator/content/bindings/notification.xml#browser-notificationbox"); >+} >+ Whoops, this belongs in communicator.css - sidebars in other windows break :-(