Add SeaMonkey support for the Windows 7 jump list

RESOLVED FIXED in seamonkey2.1final

Status

defect
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: mcsmurf, Assigned: mcsmurf)

Tracking

(Blocks 1 bug)

Trunk
seamonkey2.1final
x86
Windows 7
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.1 wanted)

Details

Attachments

(2 attachments, 4 obsolete attachments)

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).
Posted patch Work in progress 1 (obsolete) — Splinter Review
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.
Assignee: nobody → bugzilla
How does this relate to bug 515734?
Blocks: 566138
Bug 566138 is the meta bug I think.
Posted patch New Patch (obsolete) — Splinter Review
Now with jump lists only, tab preview has been excluded from this patch.
Attachment #460165 - Attachment is obsolete: true
resource://gre/modules is only for toolkit modules, not for suite modules.

We don't have a private browsing service.
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). :-)
OS: Windows XP → Windows 7
> 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.
Posted patch Patch (obsolete) — Splinter Review
New patch, this one works fine.
Attachment #508294 - Attachment is obsolete: true
Posted patch PatchSplinter Review
I do not really know where the change in uninstaller.nsi is coming from, need to revert that file probably.
Attachment #514349 - Attachment is obsolete: true
Attachment #514953 - Flags: review?(neil)
(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.
Status: NEW → ASSIGNED
downloadTaskbarIntegration.jsm is already in suite/modules/. This seems to be the place to put all our javascript *modules*.
(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).
(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 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 ;-)
Attachment #514953 - Flags: review?(neil) → review+
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);
},
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. ;-)
> >+    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?
Posted patch Patch for checkin (obsolete) — Splinter Review
Moved the module to suite/modules.
Moved the module to suite/modules/.
Attachment #521922 - Attachment is obsolete: true
Attachment #521923 - Attachment is patch: true
Attachment #521923 - Attachment mime type: application/octet-stream → text/plain
(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)?
Whops Frank forgot to resolve. (I am assuming there is no more remaining work).

c-set landed before comm-2.0 branching
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
Depends on: 747774
You need to log in before you can comment on or make changes to this bug.