Last Comment Bug 581526 - Add SeaMonkey support for the Windows 7 jump list
: Add SeaMonkey support for the Windows 7 jump list
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: OS Integration (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: seamonkey2.1final
Assigned To: Frank Wein [:mcsmurf]
:
:
Mentors:
Depends on: 747774
Blocks: 566138
  Show dependency treegraph
 
Reported: 2010-07-23 12:22 PDT by Frank Wein [:mcsmurf]
Modified: 2012-04-22 11:13 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wanted


Attachments
Work in progress 1 (58.98 KB, patch)
2010-07-25 12:17 PDT, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review
New Patch (40.45 KB, patch)
2011-01-30 14:56 PST, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review
Patch (38.79 KB, patch)
2011-02-22 16:09 PST, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review
Patch (38.77 KB, patch)
2011-02-24 16:54 PST, Frank Wein [:mcsmurf]
neil: review+
Details | Diff | Splinter Review
Patch for checkin (36.74 KB, patch)
2011-03-25 13:20 PDT, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review
Patch for checkin (36.45 KB, patch)
2011-03-25 13:21 PDT, Frank Wein [:mcsmurf]
no flags Details | Diff | Splinter Review

Description Frank Wein [:mcsmurf] 2010-07-23 12:22:57 PDT
See Bug 518666 for the bits Firefox added, see Attachment 402656 [details] for a screenshot of the UI (the menu on the right is the new jump list).
Comment 1 Frank Wein [:mcsmurf] 2010-07-25 12:17:59 PDT
Created attachment 460165 [details] [diff] [review]
Work in progress 1

This one ports Bug 518666, Bug 521141 and Bug 562753 to SeaMonkey. Also includes tab preview on the taskbar, did not test that one yet (lack of Windows 7 with Aero theme). The jump list works.
Comment 2 Robert Kaiser 2010-07-27 06:35:42 PDT
How does this relate to bug 515734?
Comment 3 Philip Chee 2010-07-27 06:42:07 PDT
Bug 566138 is the meta bug I think.
Comment 4 Frank Wein [:mcsmurf] 2011-01-30 14:56:33 PST
Created attachment 508294 [details] [diff] [review]
New Patch

Now with jump lists only, tab preview has been excluded from this patch.
Comment 5 neil@parkwaycc.co.uk 2011-01-31 03:59:32 PST
resource://gre/modules is only for toolkit modules, not for suite modules.

We don't have a private browsing service.
Comment 6 Jens Hatlak (:InvisibleSmiley) 2011-01-31 16:04:53 PST
Hmm, I wanted to try this but cannot get it to work (compared with Minefield). I get this in the Error Console (in line with comment 5):

Error: _privateBrowsingSvc is not defined
Source File: resource://gre/modules/WindowsJumpLists.jsm
Line: 180

My env is Win7 x64 with Aero (32-bit SM), VS9 SP1, Win7 SDK, Server 2003 R2 Platform SDK, MozillaBuild 1.5.1.

Feel free to request feedback from me if you have something that works for you (or if you cannot test yourself, if you're confident). :-)
Comment 7 Philip Chee 2011-01-31 19:40:06 PST
> Error: _privateBrowsingSvc is not defined
> Source File: resource://gre/modules/WindowsJumpLists.jsm
> Line: 180

From Comment 5:
> We don't have a private browsing service.
Comment 8 Frank Wein [:mcsmurf] 2011-02-22 16:09:11 PST
Created attachment 514349 [details] [diff] [review]
Patch

New patch, this one works fine.
Comment 9 Frank Wein [:mcsmurf] 2011-02-24 16:54:28 PST
Created attachment 514953 [details] [diff] [review]
Patch

I do not really know where the change in uninstaller.nsi is coming from, need to revert that file probably.
Comment 10 Ian Neal 2011-02-24 17:14:06 PST
(In reply to comment #9)
> Created attachment 514953 [details] [diff] [review]
> Patch
> 
> I do not really know where the change in uninstaller.nsi is coming from, need
> to revert that file probably.

Just wondering if suite/wintaskbar is the right level or whether it needs to be deeper within the existing structure.
Comment 11 Philip Chee 2011-02-24 17:52:20 PST
downloadTaskbarIntegration.jsm is already in suite/modules/. This seems to be the place to put all our javascript *modules*.
Comment 12 Jens Hatlak (:InvisibleSmiley) 2011-02-24 23:26:59 PST
(In reply to comment #9)
> Created attachment 514953 [details] [diff] [review]
> Patch
> 
> I do not really know where the change in uninstaller.nsi is coming from, need
> to revert that file probably.

It's a line end change (CRLF->LF). I know because it didn't apply correctly. Maybe something to be fixed through a "no bug" checkin.

BTW: This only started working for me with an existing profile when I manually ran the code you added to nsSuiteGlue.js in Execute JS (Error Console should do, too, with some reformatting). I have neither checked with a fresh profile yet nor do I know whether this makes a difference now (i.e. whether the enabling is somehow per application and remembered by the OS).
Comment 13 neil@parkwaycc.co.uk 2011-02-25 04:24:38 PST
(In reply to comment #12)
> It's a line end change (CRLF->LF). I know because it didn't apply correctly.
I'd done a sweep of line ending changes, but I didn't realise an .nsi was text :-(
Comment 14 neil@parkwaycc.co.uk 2011-02-25 04:37:58 PST
Comment on attachment 514953 [details] [diff] [review]
Patch

>+    /**
>+     * Provides the shell service an opportunity to do some Win7+ shortcut
>+     * maintenance needed on initial startup of the browser.
>+     */
>+    void shortcutMaintenance();
[It should be possible to write this in JS, using nsIProcess.]

>+nsresult
>+GetHelperPath(nsAutoString& aPath)
nsCString& (sorry I didn't notice you had used nsAutoString& twice)

>+EXTRA_JS_MODULES = \
>+	WindowsJumpLists.jsm \
>+	$(NULL)
Might as well move this to suite/modules to stop it being lonely ;-)
Comment 15 neil@parkwaycc.co.uk 2011-02-25 05:03:08 PST
Comment on attachment 514953 [details] [diff] [review]
Patch

Untested, obviously.

_shortcutMaintenance: function WTBJL__maintenance() {
  const nsISupportsString = Components.interfaces.nsISupportsString;

  var appId = _taskbarService.defaultGroupId;
  try {
    if (Services.prefs.getComplexValue(PREF_TASKBAR_LASTGROUPID,
                                       nsISupportsString).data == appId)
      return;
  } catch (e) {}

  var str = Components.classes['@mozilla.org/supports-string;1']
                      .createInstance(nsISupportsString);
  str.data = appId;
  Services.prefs.setComplexValue(PREF_TASKBAR_LASTGROUPID,
                                 nsISupportsString, str);

  var helper = Services.dirsvc.get("CurProcD",
                                   Components.interfaces.nsILocalFile);
  helper.append("uninstall");
  helper.append("helper.exe");

  var process = Components.classes["@mozilla.org/process/util;1"]
                          .createInstance(Components.interfaces.nsIProcess);
  process.init(helper);
  process.runAsync([" /UpdateShortcutAppUserModelIds"], 1);
},
Comment 16 Robert Kaiser 2011-02-25 05:49:10 PST
I'd think the Firefox guys would be quite happy to see that backported to them, they dearly love doing stuff in JS instead of C++ anyhow. ;-)
Comment 17 Philip Chee 2011-03-14 03:42:53 PDT
> >+    void shortcutMaintenance();
> [It should be possible to write this in JS, using nsIProcess.]
What say we get this checked in and push off rewriting this bit in JS to a followup bug?
Comment 18 Frank Wein [:mcsmurf] 2011-03-25 13:20:01 PDT
Created attachment 521922 [details] [diff] [review]
Patch for checkin

Moved the module to suite/modules.
Comment 19 Frank Wein [:mcsmurf] 2011-03-25 13:21:16 PDT
Created attachment 521923 [details] [diff] [review]
Patch for checkin

Moved the module to suite/modules/.
Comment 20 Frank Wein [:mcsmurf] 2011-03-25 13:29:57 PDT
(In reply to comment #14)
> >+nsresult
> >+GetHelperPath(nsAutoString& aPath)
> nsCString& (sorry I didn't notice you had used nsAutoString& twice)

You meant nsString& here? Then I fixed this (forgot to change this, will be in the checkin)?
Comment 21 Frank Wein [:mcsmurf] 2011-03-25 13:43:54 PDT
Checked in: http://hg.mozilla.org/comm-central/rev/cdaca9df5057
Comment 22 Justin Wood (:Callek) 2011-05-07 11:03:40 PDT
Whops Frank forgot to resolve. (I am assuming there is no more remaining work).

c-set landed before comm-2.0 branching

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