Closed Bug 629275 Opened 9 years ago Closed 9 years ago

Recent nightly kills Win7 Taskbar Jumplists/Tasks

Categories

(Firefox :: Shell Integration, defect)

x86
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b11
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: KWierso, Assigned: jimm)

References

Details

(Keywords: regression)

Attachments

(1 file)

After updating to today's trunk nightly, I no longer see anything special when I right-click the Taskbar icon for Minefield. Prior to updating, it showed Frequent sites and a few tasks like "Enter Private Browsing".
All related prefs in about:config (browser.taskbar.lists.*) are set to true.

I've tried unpinning and repinning the taskbar icon, but that didn't fix it. I even did a clean reinstall of the nightly with a fresh profile and it didn't fix it.
I do have multiple Firefox icons in the taskbar, for various versions. (3.6, 4.0b10, 32-bit nightly, 64-bit nightly)
Only 4.0b10's icon displays the tasks/jumplist items. Do they only work on one shortcut at any given time?
Jumplist don't work properly after bug 550392 and bug 623287 landed.  I don't
understand why the patches would have done anything to Jumplist creation.  But
this is what I found, I checked more than a few times that my jumplists don't
work:

regression range:

works:
Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110125 Firefox/4.0b10pre
ID:20110125122636
http://hg.mozilla.org/mozilla-central/rev/0fb025a84958

broken:
Mozilla/5.0 (Windows NT 6.1; rv:2.0b11pre) Gecko/20110125 Firefox/4.0b11pre
ID:20110125132516
http://hg.mozilla.org/mozilla-central/rev/5daf5e03b8e1

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0fb025a84958&tochange=5daf5e03b8e1
blocking2.0: --- → ?
Duplicate of this bug: 629279
Keywords: regression
That regression range can't be right; this is a Windows 7 issue and the only code change there is to do with OSX. :/

Jimm: can you please take a look at this and see if there's any recent changes that might have caused it?
Looks like previews in the taskbar are working so the id on the shortcut should be correct. Uninstalling then installing didn't fix it as well... strange.
(In reply to comment #4)
> That regression range can't be right; this is a Windows 7 issue and the only
> code change there is to do with OSX. :/
 
I came to the same conclusion.  It might that it had something to do with hourly archives I had to use to hunt it down, since our normal hourlies for that range had been removed already.  It certainly didn't seem right given the patch range nor did much else that landed prior to that.
Hmm, a jump list builder isn't getting created two minutes in, this should be created by the module up in browser. That gets triggered here - 

http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#363

So either that isn't firing or something in the module fails before the list is initialized.
Is it related to the version bump to b11pre, perhaps?

Also, is browser.taskbar.lastgroupid used for anything anymore? MXR only comes up with http://mxr.mozilla.org/mozilla-central/source/browser/components/shell/src/nsWindowsShellService.cpp#346
Jim, I tried to forced it to regenerate by calling
const Ci = Components.interfaces; const Cc = Components.classes; 
Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIBrowserGlue).QueryInterface(Ci.nsIObserver).observe(null, "browser-delayed-startup-finished", null);

that caused the following error

Error: uncaught exception: [Exception... "'[JavaScript Error: "enum is a reserved identifier" {file: "resource://gre/modules/WindowsJumpLists.jsm" line: 500 column: 8 source: "    var enum = items.enumerate();
"}]' when calling method: [nsIObserver::observe]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: javascript:%20const%20Ci%20=%20Components.interfaces;%20const%20Cc%20=%20Components.classes;%20%20Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIBrowserGlue).QueryInterface(Ci.nsIObserver).observe(null,%20"browser-delayed-startup-finished",%20null); :: <TOP_LEVEL> :: line 1"  data: yes]
Something changed related to js enum, I get an exception during the import of the module:

SyntaxError: enum is a reserved identifier
> 500     var enum = items.enumerate();

The fact that we used to not throw on that was a bug.  It got fixed recently.  See bug 497869.

But we should only throw in strict mode.  Is this jsm in strict mode for some reason?
Blocks: 497869
Oh, the "strict mode only" thing hasn't merged to m-c yet...  Can we get that to happen?
(In reply to comment #8)
> Is it related to the version bump to b11pre, perhaps?
No

> Also, is browser.taskbar.lastgroupid used for anything anymore? MXR only comes
> up with
> http://mxr.mozilla.org/mozilla-central/source/browser/components/shell/src/nsWindowsShellService.cpp#346
It is.
Attached patch patchSplinter Review
Also fixes a little problem with local builds, shortcutMaintenance was throwing. Went ahead and fixed that too.
Assignee: nobody → jmathies
Attachment #507526 - Flags: review?(robert.bugzilla)
Attachment #507526 - Flags: review?(robert.bugzilla) → review+
blocking2.0: ? → betaN+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/9f2831978b1c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b11
(In reply to comment #11)
> > 500     var enum = items.enumerate();
> 
> The fact that we used to not throw on that was a bug.  It got fixed recently. 
> See bug 497869.
> 
> But we should only throw in strict mode.  Is this jsm in strict mode for some
> reason?

Actually, enum is one of the keywords where we *should* throw even outside strict mode:

FutureReservedWord :: one of
class
const
enum
export
extends
import
super

But it's too late this cycle to make that change, so that got backed out slightly.  Note that IE8 and IE9 and fairly recent WebKit (whatever's in the latest Epiphany browser I'm using that optionally ships with Fedora 14) properly treat all these words as syntax errors.  Chrome and Opera (and us) are wrong in not treating these as syntax errors.

These are the keywords which are syntax errors only in strict mode (let/yield we exempt if they're in JS which opted into support for them, which all chrome JS does -- in normal web JS they're properly errors):

implements
interface
let
package
private
protected
public
static
yield
OS: Windows 7 → Windows XP
"in normal web JS" by which I meant "in strict mode JS from a <script> without a type="JS1.7" creature in it", to clear up any potential confusion.

Also, because const has been around without versioning for so long, even when we do get around to making enum and friends reserved, we will all but certainly exempt const.  Hopefully by then it'll have standardized semantics we're fine with exposing even to code predating that standardization, but you never know.
OS: Windows XP → Windows 7
> Actually, enum is one of the keywords where we *should* throw even outside

My "should" was based on reading the patches in bug 497869; I just hadn't realized only some of them had made it to m-c.
OS: Windows 7 → Windows XP
OS: Windows XP → Windows 7
Verified fixed using a tinderbox build
Status: RESOLVED → VERIFIED
This does not fix the start menu jumplist. :(
(In reply to comment #20)
> This does not fix the start menu jumplist. :(

The first refresh of the lists happens about two minutes after you start the browser. Can you confirm it's not working after installing the latest nightly and waiting for two minutes?
(In reply to comment #21)
My browser has been running for 15 minutes and I don't see any change in start menu though the taskbar list is working.
(In reply to comment #22)
> (In reply to comment #21)
> My browser has been running for 15 minutes and I don't see any change in start
> menu though the taskbar list is working.

Sorry, it works. I missed it.
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > My browser has been running for 15 minutes and I don't see any change in start
> > menu though the taskbar list is working.
> 
> Sorry, it works. I missed it.

great! thanks for testing.
You need to log in before you can comment on or make changes to this bug.