300412 - A menu option for Bookmark All Tabs and bookmark options in the right-click menu for tabs

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, enhancement)

Description

[gsshih]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b3) Gecko/20050701 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b3) Gecko/20050701 Firefox/1.0+

Under Bookmarks in the menu bar, a separate option for "Bookmark All Tabs..."
should be included.  This gets rid of the need for the checkbox "Bookmark all
tabs in folder".  Also, it would be convenient to have "Bookmark this tab" and
"Bookmark all tabs" options when right-clicking on a tab.

Reproducible: Always

Comment 1

[gsshih]

Attachment: patch for bookmark all tabs

Comment 2

[gsshih]

Comment on attachment 188991 [details] [diff] [review]
patch for bookmark all tabs

>Index: base/content/browser-menubar.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-menubar.inc,v
>retrieving revision 1.53
>diff -u -u -p -r1.53 browser-menubar.inc
>--- base/content/browser-menubar.inc	23 Jun 2005 02:25:04 -0000	1.53
>+++ base/content/browser-menubar.inc	11 Jul 2005 23:33:16 -0000
>@@ -371,6 +371,11 @@
>                           label="&manBookmarksCmd.label;"
>                           accesskey="&manBookmarksCmd.accesskey;"    
>                           oncommand="toOpenWindowByType('bookmarks:manager', 'chrome://browser/content/bookmarks/bookmarksManager.xul');"/>
>+                <menuitem id="bookmarkAllCmd"
>+                          key="addBookmarkGroupKb"
>+                          label="&bookmarkAllCmd.label;"
>+                          accesskey="&bookmarkAllCmd.accesskey;"
>+                          command="Browser:AddBookmarkGroup"/>
>                 <menuseparator/>
>               </menupopup>
>             </menu>
>Index: base/content/browser-sets.inc
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser-sets.inc,v
>retrieving revision 1.49
>diff -u -u -p -r1.49 browser-sets.inc
>--- base/content/browser-sets.inc	20 Jun 2005 06:39:22 -0000	1.49
>+++ base/content/browser-sets.inc	11 Jul 2005 23:33:16 -0000
>@@ -149,6 +149,7 @@
>     <command id="cmd_findAgain" oncommand="onFindAgainCmd();"/>
>     <command id="cmd_findPrevious" oncommand="onFindPreviousCmd();"/>    
>     <command id="Browser:AddBookmarkAs" oncommand="addBookmarkAs(document.getElementById('content'));"/>
>+    <command id="Browser:AddBookmarkGroup" oncommand="addBookmarkGroup(document.getElementById('content'));"/>
>     <command id="Browser:Home"    oncommand="BrowserHome();"/>
>     <command id="Browser:Back"    oncommand="BrowserBack();" disabled="true"/>
>     <command id="Browser:Forward" oncommand="BrowserForward();" disabled="true"/>
>Index: base/content/browser.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser.js,v
>retrieving revision 1.453
>diff -u -u -p -r1.453 browser.js
>--- base/content/browser.js	7 Jul 2005 19:30:09 -0000	1.453
>+++ base/content/browser.js	11 Jul 2005 23:33:16 -0000
>@@ -183,6 +183,17 @@ function UpdateBackForwardButtons()
>   }
> }
> 
>+function UpdateBookmarkAllTabsMenuitem()
>+{
>+  var browser = getBrowser();
>+  var numtabs = browser.tabContainer.childNodes.length;
>+  var bookmarkAllCommand = document.getElementById("Browser:AddBookmarkGroup");
>+  if (numtabs > 1)
>+    bookmarkAllCommand.setAttribute("disabled", "false");
>+  else
>+    bookmarkAllCommand.setAttribute("disabled", "true");
>+}
>+
> const gSessionHistoryObserver = {
>   observe: function(subject, topic, data)
>   {
>@@ -1524,6 +1535,32 @@ function addBookmarkForTabBrowser(aTabBr
>   var currentTabInfo = { name: "", url: "", charset: null };
> 
>   const activeBrowser = aTabBrowser.selectedBrowser;
>+
>+  var webNav = activeBrowser.webNavigation;
>+  var url = webNav.currentURI.spec;
>+  var name = "";
>+  var charSet, description;
>+  try {
>+    var doc = webNav.document;
>+    name = doc.title || url;
>+    charSet = doc.characterSet;
>+    description = BookmarksUtils.getDescriptionFromDocument(doc);
>+  } catch (e) {
>+    name = url;
>+  }
>+  
>+  currentTabInfo = {name: name, url: url, charset: charSet, description: description, bAddGroup: false};
>+  var dialogArgs = currentTabInfo;
>+  dialogArgs.objGroup = tabsInfo;
>+  openDialog("chrome://browser/content/bookmarks/addBookmark2.xul", "",
>+             BROWSER_ADD_BM_FEATURES, dialogArgs);
>+}
>+
>+function addBookmarkGroup(aTabBrowser, aSelect)
>+{
>+  var tabsInfo = [];
>+  var currentTabInfo = { name: "", url: "", charset: null };
>+  const activeBrowser = aTabBrowser.selectedBrowser;
>   const browsers = aTabBrowser.browsers;
>   for (var i = 0; i < browsers.length; ++i) {
>     var webNav = browsers[i].webNavigation;
>@@ -1538,12 +1575,14 @@ function addBookmarkForTabBrowser(aTabBr
>     } catch (e) {
>       name = url;
>     }
>-    tabsInfo[i] = { name: name, url: url, charset: charSet, description: description, bAddGroup: true };
>+    tabsInfo[i] = { name: name, url: url, charset: charSet, description: description};
>     if (browsers[i] == activeBrowser)
>       currentTabInfo = tabsInfo[i];
>   }
>+  currentTabInfo = {name: "[Folder name]"};
>   var dialogArgs = currentTabInfo;
>   dialogArgs.objGroup = tabsInfo;
>+  dialogArgs.bAddBookmarkGroup = true;
>   openDialog("chrome://browser/content/bookmarks/addBookmark2.xul", "",
>              BROWSER_ADD_BM_FEATURES, dialogArgs);
> }
>@@ -3112,7 +3151,7 @@ nsBrowserStatusHandler.prototype =
>     this.statusTextField        = document.getElementById("statusbar-display");
>     this.securityButton         = document.getElementById("security-button");
>     this.urlBar                 = document.getElementById("urlbar");
>-
>+  
>     // Initialize the security button's state and tooltip text
>     var securityUI = getBrowser().securityUI;
>     this.onSecurityChange(null, null, securityUI.state);
>@@ -3130,7 +3169,7 @@ nsBrowserStatusHandler.prototype =
>     this.securityButton         = null;
>     this.urlBar                 = null;
>     this.statusText             = null;
>-    this.lastURI                = null;
>+    this.lastURI                = null; 
>   },
> 
>   setJSStatus : function(status)
>@@ -3228,7 +3267,7 @@ nsBrowserStatusHandler.prototype =
>         if (aWebProgress.DOMWindow == content) {
>           if (aRequest)
>             this.endDocumentLoad(aRequest, aStatus);
>-          var browser = gBrowser.mCurrentBrowser;
>+          var browser = gBrowser.mCurrentBrowser;         
>           if (!gBrowser.mTabbedMode && !browser.mIconURL)
>             gBrowser.useDefaultIcon(gBrowser.mCurrentTab);
>           if (browser.mIconURL)
>@@ -3388,7 +3427,7 @@ nsBrowserStatusHandler.prototype =
>       }
>     }
>     UpdateBackForwardButtons();
>-
>+    UpdateBookmarkAllTabsMenuitem();
>     if (findField && gFindMode != FIND_NORMAL) {
>       // Close the Find toolbar if we're in old-style TAF mode
>       closeFindBar();
>Index: components/bookmarks/content/addBookmark2.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/bookmarks/content/addBookmark2.js,v
>retrieving revision 1.34
>diff -u -u -p -r1.34 addBookmark2.js
>--- components/bookmarks/content/addBookmark2.js	19 Apr 2005 15:17:04 -0000	1.34
>+++ components/bookmarks/content/addBookmark2.js	11 Jul 2005 23:33:16 -0000
>@@ -52,36 +52,38 @@
>  *   Apart from the standard openDialog parameters, this dialog can 
>  *   be passed additional information, which is contained in the 
>  *   wArg object:
>- * 
>- *   wArg.name          : Bookmark Name. The value to be prefilled
>- *                        into the "Name: " field (if visible).
>- *   wArg.description   : Bookmark description. The value to be added
>- *                      : to the boomarks description field.
>- *   wArg.url           : Bookmark URL: The location of the bookmark.
>- *                        The value to be filled in the "Location: "
>- *                        field (if visible).
>- *   wArg.folderURI     : Bookmark Folder. The RDF Resource URI of the
>- *                        folder that this bookmark should be created in.
>- *   wArg.charset       : Bookmark Charset. The charset that should be
>- *                        used when adding a bookmark to the specified
>- *                        URL. (Usually the charset of the current 
>- *                        document when launching this window).
>- *   wArg.bAddGroup     : True if adding a group of tabs, false
>- *                        otherwise.
>- *   wArg.objGroup[]    : If adding a group of tabs, this is an array
>- *                        of wArg objects with name, URL and charset
>- *                        properties, one for each group member.
>- *   wArg.bWebPanel     : If the bookmark should become a web panel.
>- *   wArg.keyword       : A suggested keyword for the bookmark. If this
>- *                        argument is supplied, the keyword row is made
>- *                        visible.
>- *   wArg.bNeedKeyword  : Whether or not a keyword is required to add
>- *                        the bookmark.
>- *   wArg.postData      : PostData to be saved with this bookmark, 
>- *                        in the format a string of name=value pairs
>- *                        separated by CRLFs.
>- *   wArg.feedURL       : feed URL for Livemarks (turns bookmark
>- *                        into Livemark)
>+ *  
>+ *   wArg.name              : Bookmark Name. The value to be prefilled
>+ *                            into the "Name: " field (if visible).
>+ *   wArg.description       : Bookmark description. The value to be added
>+ *                          : to the boomarks description field.
>+ *   wArg.url               : Bookmark URL: The location of the bookmark.
>+ *                            The value to be filled in the "Location: "
>+ *                            field (if visible).
>+ *   wArg.folderURI         : Bookmark Folder. The RDF Resource URI of the
>+ *                            folder that this bookmark should be created in.
>+ *   wArg.charset           : Bookmark Charset. The charset that should be
>+ *                            used when adding a bookmark to the specified
>+ *                            URL. (Usually the charset of the current 
>+ *                            document when launching this window).
>+ *   wArg.bAddGroup         : True if adding a group of tabs, false
>+ *                            otherwise.
>+ *   wArg.bAddBookmarkGroup : True if "Bookmark All Tabs" option is chosen,
>+ *                            false otherwise.
>+ *   wArg.objGroup[]        : If adding a group of tabs, this is an array
>+ *                            of wArg objects with name, URL and charset
>+ *                            properties, one for each group member.
>+ *   wArg.bWebPanel         : If the bookmark should become a web panel.
>+ *   wArg.keyword           : A suggested keyword for the bookmark. If this
>+ *                            argument is supplied, the keyword row is made
>+ *                            visible.
>+ *   wArg.bNeedKeyword      : Whether or not a keyword is required to add
>+ *                            the bookmark.
>+ *   wArg.postData          : PostData to be saved with this bookmark, 
>+ *                            in the format a string of name=value pairs
>+ *                            separated by CRLFs.
>+ *   wArg.feedURL           : feed URL for Livemarks (turns bookmark
>+ *                            into Livemark)
>  */
> 
> var gSelectedFolder;
>@@ -119,6 +121,7 @@ function Startup()
>   gName.focus();
>   gSuggestedKeyword = gArg.keyword;
>   gKeywordRequired = gArg.bNeedKeyword;
>+
>   if (!gSuggestedKeyword && !gKeywordRequired) {
>     gKeywordRow.hidden = true;
>   } else {
>@@ -127,6 +130,7 @@ function Startup()
>     if (gKeywordRequired)
>       gRequiredFields.push(gKeyword);
>   }
>+
>   if (!gArg.bAddGroup)
>     gGroup.setAttribute("hidden", "true");
>   sizeToContent();
>@@ -173,6 +177,9 @@ function onOK()
> 
>   var url, rSource;
>   var livemarkFeed = gArg.feedURL;
>+  if (gArg.bAddBookmarkGroup){
>+    gGroup.checked = true;
>+  }
>   if (gGroup && gGroup.checked) {
>     rSource = BMDS.createFolder(gName.value);
>     const groups = gArg.objGroup;
>Index: components/bookmarks/content/addBookmark2.xul
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/bookmarks/content/addBookmark2.xul,v
>retrieving revision 1.19
>diff -u -u -p -r1.19 addBookmark2.xul
>--- components/bookmarks/content/addBookmark2.xul	2 Mar 2005 10:45:51 -0000	1.19
>+++ components/bookmarks/content/addBookmark2.xul	11 Jul 2005 23:33:16 -0000
>@@ -55,7 +55,7 @@
>         xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
>         ondialogextra2="newFolder()"
>         ondialogaccept="return onOK(event)"
>-        buttons="accept,cancel" 
>+        buttons="accept,cancel"
>         buttonlabelextra2="&button.newfolder.label;" buttonaccesskeyextra2="&button.newfolder.accesskey;"
> #ifdef XP_UNIX
>         buttonlabelaccept="&acceptButton.label;"
>Index: locales/en-US/chrome/browser/browser.dtd
>===================================================================
>RCS file: /cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v
>retrieving revision 1.20
>diff -u -u -p -r1.20 browser.dtd
>--- locales/en-US/chrome/browser/browser.dtd	23 Jun 2005 02:25:06 -0000	1.20
>+++ locales/en-US/chrome/browser/browser.dtd	11 Jul 2005 23:33:19 -0000
>@@ -63,6 +63,8 @@
> <!ENTITY addCurPageAsCmd.commandkey "d">
> <!ENTITY manBookmarksCmd.label "Manage Bookmarks...">
> <!ENTITY manBookmarksCmd.accesskey "M">
>+<!ENTITY bookmarkAllCmd.label "Bookmark All Tabs...">
>+<!ENTITY bookmarkAllCmd.accesskey "T">
> 
> <!ENTITY backCmd.label                "Back">
> <!ENTITY backCmd.accesskey            "B">
>Index: content/widgets/tabbrowser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/tabbrowser.xml,v
>retrieving revision 1.91
>diff -u -u -p -r1.91 tabbrowser.xml
>--- content/widgets/tabbrowser.xml	7 Jul 2005 15:52:49 -0000	1.91
>+++ content/widgets/tabbrowser.xml	11 Jul 2005 23:35:04 -0000
>@@ -82,6 +82,12 @@
>                           oncommand="var tabbrowser = this.parentNode.parentNode.parentNode.parentNode;
>                                      tabbrowser.removeAllTabsBut(tabbrowser.mContextTab);"/>
>             <xul:menuseparator/>
>+            <xul:menuitem label="&addCurTabAsCmd.label;" accesskey="&addCurTabAsCmd.accesskey;" 
>+                          oncommand="var tabbrowser = this.parentNode.parentNode.parentNode.parentNode;
>+                                     addBookmarkAs(getBrowserForTab(tabbrowser.mContextTab));"/>
>+            <xul:menuitem label="&addAllTabsAsCmd.label;" accesskey="&addAllTabsAsCmd.accesskey;"
>+                          command="Browser:AddBookmarkGroup"/>
>+            <xul:menuseparator/>
>             <xul:menuitem label="&closeTab.label;" accesskey="&closeTab.accesskey;"
>                           tbattr="tabbrowser-multiple"
>                           oncommand="var tabbrowser = this.parentNode.parentNode.parentNode.parentNode;
>Index: locales/en-US/chrome/global/tabbrowser.dtd
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/locales/en-US/chrome/global/tabbrowser.dtd,v
>retrieving revision 1.2
>diff -u -u -p -r1.2 tabbrowser.dtd
>--- locales/en-US/chrome/global/tabbrowser.dtd	24 Nov 2004 15:55:24 -0000	1.2
>+++ locales/en-US/chrome/global/tabbrowser.dtd	11 Jul 2005 23:35:04 -0000
>@@ -10,3 +10,7 @@
> <!ENTITY  reloadTab.label        "Reload Tab">
> <!ENTITY  reloadTab.accesskey         "r">
> <!ENTITY  newTabButton.tooltip   "Open a new tab">
>+<!ENTITY  addCurTabAsCmd.label "Bookmark This Tab...">
>+<!ENTITY  addCurTabAsCmd.accesskey "B">
>+<!ENTITY  addAllTabsAsCmd.label "Bookmark All Tabs...">
>+<!ENTITY  addAllTabsAsCmd.accesskey "T">

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 188991 [details] [diff] [review]
patch for bookmark all tabs

hellooo bogus dependencies.  Toolkit bits cannot call functions in /browser,
its app-independent, and I don't want to add more items to the tab context menu
that aren't tab management-related, regardless.

As for the added menuitem in the bookmarks menu, that's probably necessary, but
that's a dupe to something much older, but we can ignore that for now.	I'm not
sure addbookmarkgroup is necessary in addition to addgroup if we're removing
the checkbox from the normal dialog, but I haven't done more than skim the
patch.

As a note, before you write a major UI patch, its usually a good idea to talk
to the app/module owners about how/if it should be implemented.

Comment 4

[Mike Connor [:mconnor]]

WONTFIX (as filed)

If you're still interested in the added menuitem for the bookmarks menu, please
find the original bug and attach a patch to just add that.

Comment 5

[gsshih]

Attachment: patch for bookmark all tabs and tabs context menu

Comment 6

[gsshih]

Attachment: updated patch for bookmark all tabs

Comment 7

[gsshih]

Attachment: updated patch for bookmark all tabs

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

Hi Mike. I am the module owner, and I talked to Grace about this :-)

Comment 9

[Asaf Romano (gone)]

*** This bug has been marked as a duplicate of 267785 ***

Comment 10

[Asaf Romano (gone)]

*** Bug 267785 has been marked as a duplicate of this bug. ***

Comment 11

[Mike Connor [:mconnor]]

Ben, that's all well and good, but the patch is unacceptable since it adds a bad
dependency.  It'd be nice if you actually talked to others about this type of
thing as well.

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 191361 [details] [diff] [review]
updated patch for bookmark all tabs

>+  if (numtabs > 1)
>+    bookmarkAllCommand.setAttribute("disabled", "false");
>+  else
>+    bookmarkAllCommand.setAttribute("disabled", "true");

Style nit: use interCaps for things like numTabs
Also, convention is to remove "disabled" attributes rather than set them to
false. 


>+
>+  var webNav = activeBrowser.webNavigation;
>+  var url = webNav.currentURI.spec;
>+  var name = "";
>+  var charSet, description;
> ... 

Do you really need to duplicate this entire function? Can't you make
"bookmark-all-tabs"ness a parameter instead and just use that as a trigger to
do certain tasks/etc?

>+  currentTabInfo = {name: "[Folder name]"};

This is not localizable. Put this string into browser.properties and use
gNavigatorBundle.getString() instead. 

>+  //  bookmarkDialog = document.getElementById("addBookmarkDialog");
>+  //  bookmarkDialog.setAttribute("title", "Bookmark All Tabs");

Get rid of this. 

>     this.urlBar                 = document.getElementById("urlbar");
>-
>+  

Kill the extra whitespace. 

>-    this.lastURI                = null;
>+    this.lastURI                = null; 

Ditto. 

>-          var browser = gBrowser.mCurrentBrowser;
>+          var browser = gBrowser.mCurrentBrowser;         

Ditto.

>+  getTitle();

Call this function "initTitle" or something... getTitle implies a function that
returns a value, which it does not. 

>   gSelectedFolder = RDF.GetResource(gMenulist.selectedItem.id);
>   gExpander.setAttribute("tooltiptext", gExpander.getAttribute("tooltiptextdown"));
>   gPostData = gArg.postData;
>@@ -155,6 +161,14 @@
>   }
> } 
> 
>+function getTitle()
>+{
>+  if(gArg.bAddBookmarkGroup)
>+    document.title = document.getElementById("addBookmarkDialog").getAttribute("titlenewgroup");
>+  else
>+    document.title = document.getElementById("addBookmarkDialog").getAttribute("titlenew");
>+}

Include a stringbundle in the addBookmark2.xul file like this:

<stringbundle id="bookmarksStrings"
src="chrome://browser/locale/bookmarks/bookmarks.properties"/>

and get these strings from there instead. 

>-        title="&newBookmark.title;" title-selectFolder="&selectFolder.label;"
>+        titlenew="&newBookmark.title;" titlenewgroup="&newBookmarkGroup.title;" 

See comment above about using stringbundle for these instead. 

>+            <xul:menuitem anonid="bookmarktab" label="&addCurTabAsCmd.label;" accesskey="&addCurTabAsCmd.accesskey;"/>

Like Mike said, put an anonid on the <xul:menupopup> and discover this from the
browser code, then insert these items programmatically, rather than adding them
here in the toolkit code. 

>+<!ENTITY  addCurTabAsCmd.label "Bookmark This Tab...">
>+<!ENTITY  addCurTabAsCmd.accesskey "B">
>+<!ENTITY  addAllTabsAsCmd.label "Bookmark All Tabs...">
>+<!ENTITY  addAllTabsAsCmd.accesskey "T">

Once you've done the above, these strings should go in browser.properties

Let's get another patch that fixes these things and I'll review again.

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

Mike, we did actually discuss this with beltzner.

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

(I'm talking about UI, not implementation - grace and I did discuss how this
should be done - and the latest patch is not there yet, see my review for details). 

Comment 15

[gsshih]

Attachment: new bookmarks patch

followed review comments to fix patch

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 192145 [details] [diff] [review]
new bookmarks patch

>+  if (aBookmarkAllTabs) {
>+    dialogArgs = {name: gNavigatorBundle.getString('bookmarkAllTabsDefault')};

Style nit: spaces around the { and }
and double quotes are the convention for string literals in JavaScript files. 

That + moving the initialization to delayedStartup gets r=[e-mail]. 

I'll let you attach a new patch so I can apply it and land it for you.

Comment 17

[gsshih]

Attachment: newest bookmarks patch

Done! Thanks Ben.

Comment 18

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 192147 [details] [diff] [review]
newest bookmarks patch

>   var url, rSource;
>   var livemarkFeed = gArg.feedURL;
>+  if (gArg.bBookmarkAllTabs){
>+    gGroup.checked = true;
>+  }

Oops I didn't notice this the first time... we should just eliminate the
checkbox completely if we're never going to show it, right? 

Get rid of the checkbox from addBookmark2.xul and this code as well, then in
onOK use gArg.bBookmarkAllTabs instead of gGroup.checked to determine whether
or not to add a bookmark group.

Comment 19

[gsshih]

Attachment: removed the checkbox

Comment 20

[gsshih]

Attachment: it works now! =)

Comment 21

[gsshih]

Attachment: sorry actually this works a lot better

Comment 22

[Bryan]

If I understand this bug correctly you're adding "Bookmarks-> Bookmark All
Tabs", correct?  If so can you remove any accesskey you've created for that as
we've just recently removed the accesskeys for "Bookmark This Page" & "Manage
Bookmarks" due to bug 261985.  Any accesskey key you create will cause the same
issue that is discussed in that bug.  The root issue will be fixed after 1.5 and
the accesskeys added back at that time.

~B

Comment 23

[gsshih]

Attachment: removed accesskey for bookmark all tabs menuitem

Comment 24

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 192265 [details] [diff] [review]
removed accesskey for bookmark all tabs menuitem

r=[e-mail]

Comment 25

[Ben Goodger (use ben at mozilla dot org for email)]

b4+ since contains l10n. landed. 

Comment 26

[Asaf Romano (gone)]

At least from looking at bonsai, this wasn't checked in.

Comment 27

[PikeUK]

We now have "Bookmark This Page..." on the main menu and "Bookmark This Tab..."
on the context menu. Not sure if that inconsistency is an oversight or by design.

Comment 28

[Asaf Romano (gone)]

"Bookmark This Tab" is visible and enabled when right clicking on blank parts of
the tabbar (And, triages a JS error).

Comment 29

[Chris Cunningham]

Yay. I was beginning to worry that there were still some context menus in
Firefox which hadn't been ruined by adding in tons of pointless **** to
accidentally hit yet. Here's hoping the text selection context menu gets "add
this select text as a bookmarklet" and "send this text by email to" sooner
rather than later.

 - Chris

Comment 30

[daf]

But Now I can't Ctrl+D click a check box and bookmark all tabs, how will I do
that now? Will there be a new shortcut key? Can't you just leave the checkbox
were it was?

Comment 31

[Jesse Ruderman]

Keyboard shortcut for this feature -> bug 305955