GUI needed to toggle browser.tabs.insertRelatedAfterCurrent

RESOLVED FIXED in seamonkey2.1b1

Status

SeaMonkey
Preferences
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Manuel Reimer, Assigned: Manuel Reimer)

Tracking

Trunk
seamonkey2.1b1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Assignee: nobody → Manuel.Spam
(Assignee)

Comment 1

7 years ago
Created attachment 470459 [details] [diff] [review]
Patch to add checkbox, en-US locale and help topic

First patch. Who may review this one?
(Assignee)

Comment 2

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

Updated

7 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

7 years ago
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">
Attachment #470459 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 4

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

Comment 5

7 years ago
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.
Attachment #470459 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 6

7 years ago
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...
Attachment #470459 - Attachment is obsolete: true
Attachment #471488 - Flags: superreview?
Attachment #471488 - Flags: review?
(Assignee)

Comment 7

7 years ago
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">
Attachment #471488 - Flags: superreview?(iann_bugzilla)
Attachment #471488 - Flags: superreview?
Attachment #471488 - Flags: review?(iann_bugzilla)
Attachment #471488 - Flags: review?
(Assignee)

Comment 8

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

Comment 9

7 years ago
(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 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

7 years ago
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.
Attachment #471488 - Flags: superreview?(neil)
Attachment #471488 - Flags: superreview?(iann_bugzilla)
Attachment #471488 - Flags: review?(iann_bugzilla)
Attachment #471488 - Flags: review+
(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 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.]
Attachment #471488 - Flags: superreview?(neil) → superreview+
(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

7 years ago
(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

7 years ago
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.
(Assignee)

Comment 17

7 years ago
Created attachment 473154 [details] [diff] [review]
Final patch *without* fixing trailing whitespaces
Attachment #471488 - Attachment is obsolete: true
Attachment #473154 - Flags: superreview+
Attachment #473154 - Flags: review+
(Assignee)

Comment 18

7 years ago
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.
Attachment #473159 - Flags: superreview+
Attachment #473159 - Flags: review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(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
Keywords: checkin-needed

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b1

Updated

7 years ago
Blocks: 198963
You need to log in before you can comment on or make changes to this bug.