Last Comment Bug 702297 - Recently Closed Tabs/Windows in the Go menu are no longer disabled when empty
: Recently Closed Tabs/Windows in the Go menu are no longer disabled when empty
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- minor (vote)
: seamonkey2.8
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on:
Blocks: 599731
  Show dependency treegraph
 
Reported: 2011-11-14 09:03 PST by rsx11m
Modified: 2012-01-07 13:45 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wontfix
-
wontfix
+
fixed
fixed


Attachments
Disable Recently Closed Tabs/Windows in the Go Menu when empty. (1.24 KB, patch)
2011-11-15 22:19 PST, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Splinter Review
Disable Recently Closed Tabs/Windows in the Go menu when empty. (v2) (2.59 KB, patch)
2011-11-24 05:19 PST, Edmund Wong (:ewong)
iann_bugzilla: review+
bugspam.Callek: approval‑comm‑aurora-
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review
Removed unneccessary code from original patch. (1.03 KB, patch)
2011-12-08 01:50 PST, Edmund Wong (:ewong)
neil: review+
Details | Diff | Splinter Review

Description rsx11m 2011-11-14 09:03:33 PST
At least since the 2011-10-31 nightly trunk build, the Recently Closed Tabs and Recently Closed Windows menu items remain available even if those lists are empty. Only an empty submenu shows up when selected, no Error Console messages.
Comment 1 rsx11m 2011-11-14 09:59:44 PST
I'm seeing this in 2.5 beta 4 already, both on Windows 7 and - to a lesser extent (menu items enabled but no empty submenu) - on Linux as well.

Maybe that's what Neil was referring to in bug 599731 comment #14?

> The code to enable and disable the menus was unfortunately hidden away at
> the end of updateCloseItems. This needs to be moved into a new function so
> that it can be called from the go popup.
Comment 2 rsx11m 2011-11-14 11:25:10 PST
Bug 599731 was pushed Tue Jul 12 06:52:48 2011 -0700; the 20110710 Windows comm-central build does not exhibit the problem whereas the 20110713 build does. Thus, it seems that this bug is indeed the cause of the regression but apparently was neither resolved in bug 675672 nor filed as a separate follow-up bug (which is this one here now).

Further information from Ian in bug 599731 comment #15:
> The relevant code to be moved starts at
> http://mxr.mozilla.org/comm-central/source/suite/browser/navigator.js#1500
> You will need to add a onpopupshowing to the "history-menu"
> http://mxr.mozilla.org/comm-central/source/suite/browser/navigatorOverlay.xul#380
Comment 3 Edmund Wong (:ewong) 2011-11-15 22:19:07 PST
Created attachment 574801 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go Menu when empty.
Comment 4 Jens Hatlak (:InvisibleSmiley) 2011-11-19 09:14:44 PST
This is now listed as a Known Issue starting with SM 2.5. Please try to get Aurora/Beta approval once you have r+.
Comment 5 Ian Neal 2011-11-23 06:06:46 PST
Comment on attachment 574801 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go Menu when empty.

>+function updateRecentMenuItems()
>+{
>+  var browser = getBrowser();
>+  var ss = Components.classes["@mozilla.org/suite/sessionstore;1"]
>+                     .getService(Components.interfaces.nsISessionStore);
You've added the sessionstore service here, but not removed it from earlier in updateCloseItems function.

r- for the moment
Comment 6 Edmund Wong (:ewong) 2011-11-24 05:19:47 PST
Created attachment 576730 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go menu when empty. (v2)
Comment 7 Edmund Wong (:ewong) 2011-11-29 22:22:18 PST
Pushed to http://hg.mozilla.org/comm-central/rev/1d4e052858df.
Comment 8 neil@parkwaycc.co.uk 2011-11-30 00:43:13 PST
Comment on attachment 576730 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go menu when empty. (v2)

> function updateCloseItems()
...
>+  updateRecentMenuItems();
Probably didn't need to do this here, just calling it from the Go menu suffices.
Comment 9 rsx11m 2011-12-07 12:50:35 PST
I don't know what the cutoff for 2.6 is, but are you going to request approval-aurora/beta, with or without the change Neil suggested in comment #8?
Comment 10 Edmund Wong (:ewong) 2011-12-08 01:50:19 PST
Created attachment 579986 [details] [diff] [review]
Removed unneccessary code from original patch.

v2 of the patch introduced a superfluous line.  This patch removes it and
the blank line.
Comment 11 Justin Wood (:Callek) (Away until Aug 29) 2011-12-09 14:05:32 PST
Edmund, can you please push the second patch to c-c.

IanN can you please evaluate the risk of both of these patches going into aurora and beta and handle approvals respectively?

(Code freeze for 2.6 is late monday)
Comment 12 Edmund Wong (:ewong) 2011-12-09 16:43:50 PST
Pushed to comm-central: 
http://hg.mozilla.org/comm-central/rev/64a900fced07
Comment 13 Justin Wood (:Callek) (Away until Aug 29) 2012-01-04 17:20:14 PST
Comment on attachment 576730 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go menu when empty. (v2)

Not sure how we missed this for 2.6 cycle, sorry. But we should take this for 2.7
Comment 14 Jens Hatlak (:InvisibleSmiley) 2012-01-07 13:44:08 PST
Comment on attachment 576730 [details] [diff] [review]
Disable Recently Closed Tabs/Windows in the Go menu when empty. (v2)

http://hg.mozilla.org/releases/comm-beta/rev/1430d8e9f084
Comment 15 Jens Hatlak (:InvisibleSmiley) 2012-01-07 13:44:23 PST
Comment on attachment 579986 [details] [diff] [review]
Removed unneccessary code from original patch.

http://hg.mozilla.org/releases/comm-beta/rev/7acacf881840

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