Closed Bug 393857 Opened 12 years ago Closed 12 years ago

Add an extended notificationbox to toolkit which handles notifications initiated by the containing browser internally

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: twanno, Assigned: twanno)

References

Details

Attachments

(1 file, 5 obsolete files)

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: 

Notifications of e.g. blocked popups or blocked extension installs could be handled internally by the notification box self. This will allow the code to be shared between multiple browser projects like SeaMonkey. 

   

Reproducible: Always




There is already code that handles popup blocking in browser.xml, I think it's a very natural step that when a browser is a child of a notificationbox element, the notificationbox can handle that code further for showing a notification.
Blocks: 270443, 393108, 393120
Depends on: 252830
Attached patch patch (obsolete) — Splinter Review
This patch creates an extended notificationbox which handles notifications for popup blocking and xpinstall blocking internally.

The notificationbox needs the attribute popupMenu set with the id of a <popup>, to show that menu on the button of the notification, when a popup is blocked.
  
This patch does not contain the icon to show on a notification for popup blocking. If I'm correct Firefox uses these icons currently: 
http://lxr.mozilla.org/mozilla/source/browser/themes/winstripe/browser/Info.png
http://lxr.mozilla.org/mozilla/source/browser/themes/pinstripe/browser/Info.png
So these should probably be moved to toolkit. (toolkit/themes/(p|w)instripe/global/icons/).
Attached patch patch (v2) (obsolete) — Splinter Review
This patch is the same as the previous patch updated with the addition of the new files to the corresponding jar.mn files (assuming the images mentioned in comment 1 are copied over). And it adds a preference used in this patch to the defaults.
Attachment #278419 - Attachment is obsolete: true
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch complete patch (obsolete) — Splinter Review
This patch adds internal handling of the following events to the notificationbox element:
popup blocking
xpinstall install blocking
finding missing plugins

This patch contains (minor) changes to Firefox' browser.js and toolkits pluginInstallerWizard.js. Should there also be a review from reviewers in those areas?

This patch still assumes that the images mentioned in comment 1 are copied over.

The attribute mentioned in comment 1 that needs to be set is now changed to popupmenu. 

The code concerning xpinstall blocking needs to be updated when bug 252830 gets fixed.
Attachment #278455 - Attachment is obsolete: true
Attachment #278663 - Flags: review?
Assignee: nobody → twanno
Version: unspecified → Trunk
Blocks: 278831
Version: Trunk → unspecified
Attachment #278663 - Flags: review? → review?(neil)
Comment on attachment 278663 [details] [diff] [review]
complete patch

When I mentioned this to him on IRC he wasn't convinced that we could share this but let's see...
Attachment #278663 - Flags: review?(gavin.sharp)
Attached patch complete patch (v2) (obsolete) — Splinter Review
The previous patch contained some errors I forgot to fix.

BTW Would it be disadvantageous to change the observer notification for "xpinstall-install-blocked" in an event dispatched from the originating document, with installInfo as a property (installInfo is introduced in bug 252830)?

In that way only one notificationbox would receive a notification to its event listener, while now every notificationbox gets a notification so you must check if the originating document is in the browser that lives in the notificationbox. 

And when this bug gets approval and Firefox decides to use this notificationbox, the only place where code is observing for "xpinstall-install-blocked" is here. (according to lxr)
Attachment #278663 - Attachment is obsolete: true
Attachment #278957 - Flags: review?(gavin.sharp)
Attachment #278663 - Flags: review?(neil)
Attachment #278663 - Flags: review?(gavin.sharp)
Attachment #278957 - Flags: review?(neil)
Attached patch complete patch (v3) (obsolete) — Splinter Review
Update patch with changes from bug 252830.
Attachment #278957 - Attachment is obsolete: true
Attachment #279150 - Flags: review?(neil)
Attachment #278957 - Flags: review?(neil)
Attachment #278957 - Flags: review?(gavin.sharp)
Attachment #279150 - Flags: review?(gavin.sharp)
Remove some mistakes from the previous patch.
Attachment #279150 - Attachment is obsolete: true
Attachment #279320 - Flags: review?(neil)
Attachment #279150 - Flags: review?(neil)
Attachment #279150 - Flags: review?(gavin.sharp)
Attachment #279320 - Flags: review?(gavin.sharp)
Attachment #279320 - Flags: review?(neil)
Attachment #279320 - Flags: review?(gavin.sharp)
According to gavin on IRC this is not wanted for Firefox, so I'm marking this bug as WONTFIX.

The changes needed in toolkit to be able to use the extended notificationbox in SeaMonkey will be handled in bug 400764
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.