Last Comment Bug 588122 - GUI needed to toggle browser.tabs.insertRelatedAfterCurrent
: GUI needed to toggle browser.tabs.insertRelatedAfterCurrent
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: seamonkey2.1b1
Assigned To: Manuel Reimer
:
:
Mentors:
Depends on:
Blocks: 198963
  Show dependency treegraph
 
Reported: 2010-08-17 11:48 PDT by Manuel Reimer
Modified: 2010-12-05 06:05 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to add checkbox, en-US locale and help topic (2.50 KB, patch)
2010-08-30 09:03 PDT, Manuel Reimer
iann_bugzilla: review-
Details | Diff | Splinter Review
Second patch (4.45 KB, patch)
2010-09-02 06:44 PDT, Manuel Reimer
iann_bugzilla: review+
neil: superreview+
Details | Diff | Splinter Review
Final patch *without* fixing trailing whitespaces (4.45 KB, patch)
2010-09-08 12:07 PDT, Manuel Reimer
Manuel.Spam: review+
Manuel.Spam: superreview+
Details | Diff | Splinter Review
Final patch with useless trailing whitespaces removed (8.01 KB, patch)
2010-09-08 12:09 PDT, Manuel Reimer
Manuel.Spam: review+
Manuel.Spam: superreview+
Details | Diff | Splinter Review

Description Manuel Reimer 2010-08-17 11:48:59 PDT
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 2010-08-30 09:03:47 PDT
Created attachment 470459 [details] [diff] [review]
Patch to add checkbox, en-US locale and help topic

First patch. Who may review this one?
Comment 2 Manuel Reimer 2010-08-30 09:06:05 PDT
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 2010-08-31 09:07:48 PDT
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 2010-08-31 09:08:59 PDT
Ignore last comment... I seem to be too stupid to set review request for patches...
Comment 5 Ian Neal 2010-08-31 10:01:47 PDT
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 2010-09-02 06:44:00 PDT
Created attachment 471488 [details] [diff] [review]
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 2010-09-02 06:45:01 PDT
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 2010-09-02 06:46:12 PDT
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 2010-09-02 06:48:21 PDT
(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) 2010-09-02 08:13:02 PDT
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 2010-09-04 05:13:02 PDT
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 neil@parkwaycc.co.uk 2010-09-04 07:41:05 PDT
(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 neil@parkwaycc.co.uk 2010-09-04 07:44:00 PDT
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) 2010-09-04 14:10:17 PDT
(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 2010-09-04 14:45:43 PDT
(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 2010-09-07 09:40:02 PDT
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 2010-09-08 12:07:13 PDT
Created attachment 473154 [details] [diff] [review]
Final patch *without* fixing trailing whitespaces
Comment 18 Manuel Reimer 2010-09-08 12:09:57 PDT
Created attachment 473159 [details] [diff] [review]
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) 2010-09-09 20:56:18 PDT
(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

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