Last Comment Bug 589910 - Notifications don't work in the sidebar
: Notifications don't work in the sidebar
Status: RESOLVED FIXED
: fixed-seamonkey2.0.9, regression
Product: SeaMonkey
Classification: Client Software
Component: Sidebar (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-23 13:57 PDT by neil@parkwaycc.co.uk
Modified: 2010-09-08 13:02 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (2.62 KB, patch)
2010-08-23 14:07 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
kairo: approval‑seamonkey2.0.9+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2010-08-23 13:57:50 PDT
At some point, probably soon after we implemented notifications, since I know I would have tested it at the time, and it fails in an old 2.0pre/Gecko1.9.1pre build, the <browser> element got changed such that it now has a non-null docShell property even when it is hidden.

This means that notifications in the sidebar don't actually work :-(

Steps to reproduce problem:
javascript:sidebar.addPanel('Popup Test', 'http://www.popuptest.com/popuptest1.html', null);
Comment 1 neil@parkwaycc.co.uk 2010-08-23 14:07:00 PDT
Created attachment 468444 [details] [diff] [review]
Proposed patch

I also took the opportunity to move the button(s) between the icon and the close button. This often saves space in the common case, and I used an arrowscrollbox to stop the sidebar looking too bad in the multiple button case. Geolocation is a particularly bad example, try the following:

javascript:sidebar.addPanel("Demo: geolocation", "http://html5demos.com/geo", null);

(note that this "works" in the sidebar without the patch, because the geolocation notification code accesses the notificationbox directly.)
Comment 2 Ian Neal 2010-08-24 06:16:26 PDT
Comment on attachment 468444 [details] [diff] [review]
Proposed patch

r=me
Comment 3 neil@parkwaycc.co.uk 2010-08-24 15:47:23 PDT
Pushed changeset 262c8f5ec24d to comm-central.
Comment 4 Robert Kaiser 2010-08-24 17:45:37 PDT
Comment on attachment 468444 [details] [diff] [review]
Proposed patch

I'm pretty sure we don't have any add-ons or such hooking into sidebar notifications, but else the <binding> change wouldn't be too good on a stable branch. In this case, I guess it's OK.
Comment 5 Robert Kaiser 2010-08-25 15:41:02 PDT
Comment on attachment 468444 [details] [diff] [review]
Proposed patch

2.0.7 has been shipped, transferring approval to 2.0.8 instead.
Comment 6 neil@parkwaycc.co.uk 2010-09-08 13:02:46 PDT
Pushed changeset e1bb865fb450 to releases/comm-1.9.1

Note You need to log in before you can comment on or make changes to this bug.