588122 - GUI needed to toggle browser.tabs.insertRelatedAfterCurrent

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Manuel Reimer]

We should make it possible for users to easily enable/disable "open new tab after current" in form of a checkbox in the "tabbed browser" prefpane.

Comment 1

[Manuel Reimer]

Attachment: Patch to add checkbox, en-US locale and help topic

First patch. Who may review this one?

Comment 2

[Manuel Reimer]

One note: Please tell me before checking this in. pref-tabs.xul has some useless trailing whitespaces. I could attach a patch without them.

Comment 3

[Manuel Reimer]

Comment on attachment 470459 [details] [diff] [review]
Patch to add checkbox, en-US locale and help topic

>diff --git a/suite/common/pref/pref-tabs.xul b/suite/common/pref/pref-tabs.xul
>--- a/suite/common/pref/pref-tabs.xul
>+++ b/suite/common/pref/pref-tabs.xul
>@@ -71,6 +71,9 @@
>       <preference id="browser.tabs.opentabfor.urlbar"
>                   name="browser.tabs.opentabfor.urlbar"
>                   type="bool"/>
>+      <preference id="browser.tabs.insertRelatedAfterCurrent"
>+                  name="browser.tabs.insertRelatedAfterCurrent"
>+                  type="bool"/>
>     </preferences>
> 
>       <groupbox id="generalTabPreferences" align="start">
>@@ -87,6 +90,10 @@
>                   label="&warnOnClose.label;"
>                   accesskey="&warnOnClose.accesskey;"
>                   preference="browser.tabs.warnOnClose"/>
>+        <checkbox id="tabRelatedAfterCurrent"
>+                  label="&relatedAfterCurrent.label;"
>+                  accesskey="&relatedAfterCurrent.accesskey;"
>+                  preference="browser.tabs.insertRelatedAfterCurrent"/>
>       </groupbox>
> 
>       <groupbox id="loadGroupPreferences" align="start">
>diff --git a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>--- a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>+++ b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>@@ -450,6 +450,9 @@
>       <li><strong>Warn me when closing a window with multiple tabs</strong>:
>         Select this to make &brandShortName; warn you when you try to close a
>         browser window which has multiple tabs open in it.</li>
>+      <li><strong>Open related tabs after current tab</strong>:
>+        Select this to make new tabs open next to the relative tab. When
>+        unchecked, new tabs open after the last tab on the tab bar.</li>
>     </ul>
>   </li>
>   <li><strong>When opening a bookmark group</strong>:
>diff --git a/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd b/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>--- a/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>+++ b/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>@@ -6,6 +6,8 @@
> <!ENTITY background.accesskey "S">
> <!ENTITY warnOnClose.label "Warn me when closing a window with multiple tabs">
> <!ENTITY warnOnClose.accesskey "W">
>+<!ENTITY relatedAfterCurrent.label "Open related tabs after current tab">
>+<!ENTITY relatedAfterCurrent.accesskey "f">
> <!ENTITY loadGroup.label "When opening a bookmark group">
> <!ENTITY loadGroupAppend.label "Add tabs">
> <!ENTITY loadGroupAppend.accesskey "A">

Comment 4

[Manuel Reimer]

Ignore last comment... I seem to be too stupid to set review request for patches...

Comment 5

[Ian Neal]

Comment on attachment 470459 [details] [diff] [review]
Patch to add checkbox, en-US locale and help topic

>diff --git a/suite/common/pref/pref-tabs.xul b/suite/common/pref/pref-tabs.xul
>@@ -71,6 +71,9 @@
>       <preference id="browser.tabs.opentabfor.urlbar"
>                   name="browser.tabs.opentabfor.urlbar"
>                   type="bool"/>
>+      <preference id="browser.tabs.insertRelatedAfterCurrent"
>+                  name="browser.tabs.insertRelatedAfterCurrent"
>+                  type="bool"/>
Should be added in the order they appear in the dialog, so after "browser.tabs.warnOnClose" preference.
>     </preferences>

>diff --git a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>@@ -450,6 +450,9 @@
>       <li><strong>Warn me when closing a window with multiple tabs</strong>:
>         Select this to make &brandShortName; warn you when you try to close a
>         browser window which has multiple tabs open in it.</li>
>+      <li><strong>Open related tabs after current tab</strong>:
>+        Select this to make new tabs open next to the relative tab. When
>+        unchecked, new tabs open after the last tab on the tab bar.</li>
I think you need to explain better what "next to the relative tab" means, what is the relative tab?

>diff --git a/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd b/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>@@ -6,6 +6,8 @@
> <!ENTITY background.accesskey "S">
> <!ENTITY warnOnClose.label "Warn me when closing a window with multiple tabs">
> <!ENTITY warnOnClose.accesskey "W">
>+<!ENTITY relatedAfterCurrent.label "Open related tabs after current tab">
>+<!ENTITY relatedAfterCurrent.accesskey "f">
What is wrong with using "O"?

Nit: please give a context of least 8 for patches.
r- for the moment as I want to review the better explanation.

Comment 6

[Manuel Reimer]

Attachment: Second patch

Fixed according to your last suggestions.

I tried to create a better explaination for the help system, but english is not my native language, so please don't expect a "perfect" explaination from me...

Comment 7

[Manuel Reimer]

Comment on attachment 471488 [details] [diff] [review]
Second patch

>diff --git a/suite/common/pref/pref-tabs.xul b/suite/common/pref/pref-tabs.xul
>--- a/suite/common/pref/pref-tabs.xul
>+++ b/suite/common/pref/pref-tabs.xul
>@@ -57,16 +57,19 @@
>                   type="bool"/>
>       <preference id="browser.tabs.loadInBackground"
>                   name="browser.tabs.loadInBackground"
>                   type="bool"
>                   inverted="true"/>
>       <preference id="browser.tabs.warnOnClose"
>                   name="browser.tabs.warnOnClose"
>                   type="bool"/>
>+      <preference id="browser.tabs.insertRelatedAfterCurrent"
>+                  name="browser.tabs.insertRelatedAfterCurrent"
>+                  type="bool"/>
>       <preference id="browser.tabs.loadGroup"
>                   name="browser.tabs.loadGroup"
>                   type="int"/>
>       <preference id="browser.tabs.opentabfor.middleclick"
>                   name="browser.tabs.opentabfor.middleclick"
>                   type="bool"/>
>       <preference id="browser.tabs.opentabfor.urlbar"
>                   name="browser.tabs.opentabfor.urlbar"
>@@ -82,16 +85,20 @@
>         <checkbox id="tabBackground"
>                   label="&background.label;" 
>                   accesskey="&background.accesskey;" 
>                   preference="browser.tabs.loadInBackground"/>
>         <checkbox id="tabWarnOnClose"
>                   label="&warnOnClose.label;"
>                   accesskey="&warnOnClose.accesskey;"
>                   preference="browser.tabs.warnOnClose"/>
>+        <checkbox id="tabRelatedAfterCurrent"
>+                  label="&relatedAfterCurrent.label;"
>+                  accesskey="&relatedAfterCurrent.accesskey;"
>+                  preference="browser.tabs.insertRelatedAfterCurrent"/>
>       </groupbox>
> 
>       <groupbox id="loadGroupPreferences" align="start">
>         <caption label="&loadGroup.label;"/>
>         <radiogroup id="loadGroup"
>                     orient="horizontal"
>                     preference="browser.tabs.loadGroup">
>           <radio value="0"
>diff --git a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>--- a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>+++ b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>@@ -445,16 +445,20 @@
>         this to display the Tabbed Browsing bar only when more then one
>         browser tab is open.</li>
>       <li><strong>Switch to new tabs opened from links</strong>: Select this to
>         make &brandShortName; switch to the new tab when using <q>Open in a
>         New Tab</q> to open a link.</li>
>       <li><strong>Warn me when closing a window with multiple tabs</strong>:
>         Select this to make &brandShortName; warn you when you try to close a
>         browser window which has multiple tabs open in it.</li>
>+      <li><strong>Open related tabs after current tab</strong>:
>+        Select this to make relative tabs open next to the tab, they have been
>+        opened from. When unchecked, new tabs open after the last tab on the
>+        tab bar.</li>
>     </ul>
>   </li>
>   <li><strong>When opening a bookmark group</strong>:
>     <ul>
>       <li><strong>Add tabs</strong>: Select this if you want a bookmark group
>         to be opened in new tabs.</li>
>       <li><strong>Replace existing tabs</strong>: Select this if you want a
>         bookmark group to replace your existing tabs.</li>
>diff --git a/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd b/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>--- a/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>+++ b/suite/locales/en-US/chrome/common/pref/pref-tabs.dtd
>@@ -1,14 +1,16 @@
> <!ENTITY tabHeader.label "Tabbed Browsing">
> <!ENTITY tabDisplay.label "Tab Display">
> <!ENTITY autoHide.label "Hide the tab bar when only one tab is open">
> <!ENTITY autoHide.accesskey "d">
> <!ENTITY background.label "Switch to new tabs opened from links">
> <!ENTITY background.accesskey "S">
> <!ENTITY warnOnClose.label "Warn me when closing a window with multiple tabs">
> <!ENTITY warnOnClose.accesskey "W">
>+<!ENTITY relatedAfterCurrent.label "Open related tabs after current tab">
>+<!ENTITY relatedAfterCurrent.accesskey "O">
> <!ENTITY loadGroup.label "When opening a bookmark group">
> <!ENTITY loadGroupAppend.label "Add tabs">
> <!ENTITY loadGroupAppend.accesskey "A">
> <!ENTITY loadGroupReplace.label "Replace existing tabs">
> <!ENTITY loadGroupReplace.accesskey "R">
> <!ENTITY openTabs.label "Open tabs instead of windows for">

Comment 8

[Manuel Reimer]

Why the **** does this f***ing Bugzilla post the whole patch as comment, if I just change something on the review settings...

Comment 9

[Robert Kaiser]

(In reply to comment #8)
> Why the **** does this f***ing Bugzilla post the whole patch as comment, if I
> just change something on the review settings...

Empty your browser cache, then it will work. There's something strange going on between a few-weeks-ago update of Bugzilla and browser caches.

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 471488 [details] [diff] [review]
Second patch

>+      <li><strong>Open related tabs after current tab</strong>:
>+        Select this to make relative tabs open next to the tab, they have been

I think "relative tabs" (which you would again not explain this way, and I think it would be wrong anyway) should just be "new tabs", and the comma needs to be removed.

Comment 11

[Ian Neal]

Comment on attachment 471488 [details] [diff] [review]
Second patch

>diff --git a/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml b/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml
>@@ -445,16 +445,20 @@
>         this to display the Tabbed Browsing bar only when more then one
>         browser tab is open.</li>
>       <li><strong>Switch to new tabs opened from links</strong>: Select this to
>         make &brandShortName; switch to the new tab when using <q>Open in a
>         New Tab</q> to open a link.</li>
>       <li><strong>Warn me when closing a window with multiple tabs</strong>:
>         Select this to make &brandShortName; warn you when you try to close a
>         browser window which has multiple tabs open in it.</li>
>+      <li><strong>Open related tabs after current tab</strong>:
>+        Select this to make relative tabs open next to the tab, they have been
>+        opened from. When unchecked, new tabs open after the last tab on the
>+        tab bar.</li>
As Jens said:
>+        Select this to make new tabs open next to the tab they have been
>+        opened from. When unchecked, new tabs open after the last tab on the
>+        tab bar.</li>
r=me with that change.
I cannot sr though, neil needs to do that.

Comment 12

[[e-mail]]

(In reply to comment #11)
>(From update of attachment 471488 [details] [diff] [review])
>>+        Select this to make relative tabs open next to the tab, they have been
>>+        opened from. When unchecked, new tabs open after the last tab on the
>>+        tab bar.</li>
>As Jens said:
>Select this to make new tabs open next to the tab they have been
>opened from. When unchecked, new tabs open after the last tab on the
>tab bar.</li>
I think this is missing a "that" betwen "tab" and "they". Or for real grammar pedants, "the tab from which they have been opened." ;-)

Comment 13

[[e-mail]]

Comment on attachment 471488 [details] [diff] [review]
Second patch

>       <preference id="browser.tabs.loadGroup"
>                   name="browser.tabs.loadGroup"
>                   type="int"/>
[Someone remind me to file a bug to remove the old bookmark group code.]

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #12)
> (In reply to comment #11)
> >(From update of attachment 471488 [details] [diff] [review])
> >>+        Select this to make relative tabs open next to the tab, they have been
> >>+        opened from. When unchecked, new tabs open after the last tab on the
> >>+        tab bar.</li>
> >As Jens said:
> >Select this to make new tabs open next to the tab they have been
> >opened from. When unchecked, new tabs open after the last tab on the
> >tab bar.</li>
> I think this is missing a "that" betwen "tab" and "they". Or for real grammar
> pedants, "the tab from which they have been opened." ;-)

Your first suggestion sounds wrong to me, but then I'm not a native (English) speaker but only trusting my intuition (which still tells me my version is correct). Your second suggestion sounds rights to me, too, though. I'll leave this one to Ian. ;-)

Comment 15

[Ian Neal]

(In reply to comment #14)
> (In reply to comment #12)
> > (In reply to comment #11)
> > >(From update of attachment 471488 [details] [diff] [review] [details])
> > >>+        Select this to make relative tabs open next to the tab, they have been
> > >>+        opened from. When unchecked, new tabs open after the last tab on the
> > >>+        tab bar.</li>
> > >As Jens said:
> > >Select this to make new tabs open next to the tab they have been
> > >opened from. When unchecked, new tabs open after the last tab on the
> > >tab bar.</li>
> > I think this is missing a "that" betwen "tab" and "they". Or for real grammar
> > pedants, "the tab from which they have been opened." ;-)
> 
> Your first suggestion sounds wrong to me, but then I'm not a native (English)
> speaker but only trusting my intuition (which still tells me my version is
> correct). Your second suggestion sounds rights to me, too, though. I'll leave
> this one to Ian. ;-)

The second is more precise, the first is more lazy, so I would go for the second one.

Comment 16

[Robert Kaiser]

Manuel has asked me on IRC, but I missed him, so, yes, please, attach an updated patch exactly as it is supposed to be checked in, forward r/sr, and add the checkin-needed keyword. Thanks.

Comment 17

[Manuel Reimer]

Attachment: Final patch *without* fixing trailing whitespaces

Comment 18

[Manuel Reimer]

Attachment: Final patch with useless trailing whitespaces removed

Attached both, patch with latest changes and patch without useless trailing whitespaces. If possible, check in the second one, so those ugly trailing whitespaces are dropped.

Comment 19

[Justin Wood (:Callek)]

(In reply to comment #18)
> If possible, check in the second one, 

Done: http://hg.mozilla.org/comm-central/rev/c39ce32abdda

Also for future, see: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed