The default bug view has changed. See this FAQ.

Move the Link Behaviour preferences from the tabs pane to a separate pane.

RESOLVED FIXED in seamonkey2.1a3

Status

SeaMonkey
Preferences
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.1a3

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

7 years ago
The current tabs preferences pane is overcrowded and in some themes is clipped at the bottom. We can make it less congested by moving the link behaviour preferences to a separate pane.
(Assignee)

Comment 1

7 years ago
Created attachment 450131 [details] [diff] [review]
Patch v1.0 links pane.

>diff --git a/suite/common/pref/preferences.xul b/suite/common/pref/preferences.xul
>+          <treeitem id="linksItem"
>+                    label="&links.label;"
>+                    prefpane="links_pane"
>+                    helpTopic="navigator_pref_tabbed_browsing"

Note to self: file a followup help bug.
Attachment #450131 - Flags: review?(iann_bugzilla)

Comment 2

7 years ago
Comment on attachment 450131 [details] [diff] [review]
Patch v1.0 links pane.

In principle I am happy with this but I'd like some better accesskeys in the two resultant pref panes now there is more choice.
Attachment #450131 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 3

7 years ago
Created attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

> Ian Neal      2010-06-15 13:43:14 PDT

> (From update of attachment 450131 [details] [diff] [review])
> In principle I am happy with this but I'd like some better accesskeys in the
> two resultant pref panes now there is more choice.

I changed about two access keys; the rest of the keys are pretty much optimized.

Tabbed Browsing->Open tabs instead of windows. These access keys come from platformCommunicatorOverlay.dtd and I don't want to change anything in case I break something.

Link Behaviour. The first group uses the first available consonant in (current|tab|window). The second group uses the first available vowel in (current|tab|window).
Attachment #450131 - Attachment is obsolete: true
Attachment #451589 - Flags: review?(iann_bugzilla)
Attachment #451589 - Flags: feedback?(kairo)

Comment 4

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

>+++ b/suite/locales/en-US/chrome/common/pref/pref-links.dtd
>@@ -0,0 +1,17 @@
>+<!ENTITY linksHeader.label "Link Behaviour">
>+<!ENTITY newWindow.label "Link open behavior">
>+<!ENTITY newWindowDescription.label "Open links meant to open a new window in">
>+<!ENTITY external.label "Links from other applications">
>+<!ENTITY externalDescription.label "Open links passed from other applications in">
>+
>+<!ENTITY openCurrent.label "The current tab/window">
>+<!ENTITY newWindowGroupCurrent.accesskey "c">
>+<!ENTITY externalGroupCurrent.accesskey "u">
>+
>+<!ENTITY openTab.label "A new tab in the current window">
>+<!ENTITY newWindowGroupTab.accesskey "t">
>+<!ENTITY externalGroupTab.accesskey "a">
Shouldn't this be "A"?
>+
>+<!ENTITY openWindow.label "A new window">
>+<!ENTITY newWindowGroupWindow.accesskey "w">
>+<!ENTITY externalGroupWindow.accesskey "o">
What about using "n" for one of these?

r=me with those addressed.
Attachment #451589 - Flags: review?(iann_bugzilla) → review+

Comment 5

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

I'm not really fond of this, as I think that we actually should and could win space by merging the UI prefs for internal and external links, which I believe we will need to do when we make all tabs in all components the same - but then, we are not there yet...
Attachment #451589 - Flags: feedback?(kairo)
(Assignee)

Comment 6

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

> >+<!ENTITY openTab.label "A new tab in the current window">
> >+<!ENTITY newWindowGroupTab.accesskey "t">
> >+<!ENTITY externalGroupTab.accesskey "a">
> Shouldn't this be "A"?

> >+<!ENTITY openWindow.label "A new window">
> >+<!ENTITY newWindowGroupWindow.accesskey "w">
> >+<!ENTITY externalGroupWindow.accesskey "o">
> What about using "n" for one of these?

> r=me with those addressed.

See Comment 3.

Normally this would be optimal. However the way I reverse engineered the current access keys is that the key words are (current|tab|window) so the first group uses the first available consonant from these words. And the second group selects the first available vowel.

> What about using "n" for one of these?
The "n" ends up on _n_ew and not wi_n_dow.
Attachment #451589 - Flags: superreview?(neil)
Attachment #451589 - Flags: feedback?(iann_bugzilla)

Comment 7

7 years ago
Why not just change the radiobuttons into a menulist?
+- Link open behaviour -----------------------------+
| Open links meant to open a new window in          |
|    [  A new window                     | V ]      |
| Open links passed from other applications in      |
|    [  A new tab in the current window  | V ]      |
+---------------------------------------------------+
(Assignee)

Comment 8

7 years ago
Hmm. Good Idea. This means I don't have to split the pref-tabs and I can get rid of all those pesky access keys.
(Assignee)

Comment 9

7 years ago
> Why not just change the radiobuttons into a menulist?
Actually this doesn't seem to save any significant vertical space.
(Assignee)

Comment 10

7 years ago
Created attachment 452223 [details]
Links Prefs as menulists.

>> Why not just change the radiobuttons into a menulist?
> Actually this doesn't seem to save any significant vertical space.

I think moving the links to a separate prefpane as in patch v1.1 is still a good idea as this frees up some space for newer tabbrowser prefs e.g. those features Misak is working on. Neil? IanN?
Attachment #452223 - Flags: feedback?(neil)
(Assignee)

Updated

7 years ago
Attachment #452223 - Flags: feedback?(iann_bugzilla)
(In reply to comment #10)
> I think moving the links to a separate prefpane as in patch v1.1 is still a
> good idea as this frees up some space for newer tabbrowser prefs e.g. those
> features Misak is working on. Neil? IanN?
Hmm, there are a number of tab-related prefs in browser-prefs.js which we don't expose yet. It would also give us a chance to add the diversion restriction.
(In reply to comment #11)
> It would also give us a chance to add the diversion restriction.

xref Bug 378213

Comment 13

7 years ago
(In reply to comment #11)
> (In reply to comment #10)
> > I think moving the links to a separate prefpane as in patch v1.1 is still a
> > good idea as this frees up some space for newer tabbrowser prefs e.g. those
> > features Misak is working on. Neil? IanN?
> Hmm, there are a number of tab-related prefs in browser-prefs.js which we don't
> expose yet. It would also give us a chance to add the diversion restriction.

Indeed, is it worth rolling up into this patch?

Comment 14

7 years ago
Comment on attachment 452223 [details]
Links Prefs as menulists.

I prefer a separate pref panel
Attachment #452223 - Flags: feedback?(iann_bugzilla) → feedback-

Comment 15

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

Having had another look at the wording, I think with a few tweaks could simplify things. "other applications" being repeated for example.
Attachment #451589 - Flags: feedback?(iann_bugzilla) → feedback-
(Assignee)

Comment 16

7 years ago
> Having had another look at the wording, I think with a few tweaks could
> simplify things. "other applications" being repeated for example.

Suggestions for wording?

+<!ENTITY newWindow.label "Link open behavior">
+<!ENTITY newWindowDescription.label "Open links meant to open a new window in">
+<!ENTITY external.label "Links from other applications">
+<!ENTITY externalDescription.label "Open links passed from other applications in">

How about: "Open external links in" ?
(In reply to comment #16)
> +<!ENTITY newWindow.label "Link open behavior">
> (...)
> +<!ENTITY external.label "Links from other applications">
> 
> How about: "Open external links in" ?

Or to make it more in line with the other box: "External link open behavior"?
(Assignee)

Comment 18

7 years ago
>> How about: "Open external links in" ?
> Or to make it more in line with the other box: "External link open behavior"?

I'm talking about this entity:
+<!ENTITY externalDescription.label "Open links passed from other applications in">
(In reply to comment #18)
> >> How about: "Open external links in" ?
> > Or to make it more in line with the other box: "External link open behavior"?
> 
> I'm talking about this entity:
> +<!ENTITY externalDescription.label "Open links passed from other applications
> in">

Ah, I see. Ian's comment left it open (on purpose?) whether to change the label or description. I'm still unsure which is better to be changed: "external links" is a bit too vague for my taste but as long as there is a reference to "other applications" in either the label or description I guess it's OK.

BTW: You have 2x "Behaviour" (BE) in your patch, and in this bug's summary. ;-)
(Assignee)

Updated

7 years ago
Attachment #452223 - Flags: feedback?(neil)
(Assignee)

Updated

7 years ago
Attachment #451589 - Flags: superreview?(neil)
(Assignee)

Comment 20

7 years ago
Created attachment 453028 [details]
Prefpane Screenshot: Tweak description.

> Having had another look at the wording, I think with a few tweaks could
> simplify things. "other applications" being repeated for example.

---------------------------------------
From: http://kb.mozillazine.org/Browser.link.open_external

1 Open links from external programs in the current window.
2 Open links from external programs in a new window. (Default in SeaMonkey)
3 Open links from external programs in new tabs in an existing window. (Default in Firefox)

---------------------------------------
From: Firefox 3.0

mozilla/browser/components/preferences/tabs.js

* browser.link.open_external
* - determines where pages opened by external applications are opened:
*     0 opens such links in the default window,
*     1 opens such links in the most recent window or tab,
*     2 opens such links in a new window,
*     3 opens such links in a new tab

<!ENTITY newPagesIn.label             "New pages should be opened in:">
---------------------------------------
In the attached screenshot, I changed the description in the second group from:

Open links passed from other applications in
to
New Pages sent from other programs should open in
Attachment #453028 - Flags: review?(iann_bugzilla)

Comment 21

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

I've just been thinking ahead to if/when bug 378213 gets fixed and what is the best layout to allow for that. For the moment I am happy to go with the current patch. What do you think about adding all four options or would that be too confusing?
Attachment #451589 - Flags: feedback- → feedback+

Comment 22

7 years ago
Comment on attachment 453028 [details]
Prefpane Screenshot: Tweak description.

I was thinking of changing the groupbox caption. For example for the external links one, to "When links are passed from other applications open in" and removing the description. Though, as I said, if we're looking at how the layout will change when other prefs are added, it might be better to leave those as they are.
Attachment #453028 - Flags: review?(iann_bugzilla) → review-
(Assignee)

Comment 23

7 years ago
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

> I've just been thinking ahead to if/when bug 378213 gets fixed and what is the
> best layout to allow for that. For the moment I am happy to go with the current
> patch.

Hmm.

> // 0: no restrictions - divert everything
> // 1: don't divert window.open at all
> // 2: don't divert window.open with features
> pref("browser.link.open_newwindow.restriction", 0);

Looking at:

http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsContentTreeOwner.cpp#877

> What do you think about adding all four options or would that be too
> confusing?

I think this needs some thought as to the UI. So I prefer that the restriction preference be done in a separate patch. Lets get the UI in this patch into the tree first.
Attachment #451589 - Flags: superreview?(neil)
Comment on attachment 451589 [details] [diff] [review]
Patch v1.1 Access Keys

>-            
>+
Please try to avoid random whitespace changes. This one is OK though.

>-   </prefpane>
>+  </prefpane>
As is this.

>+                    helpTopic="navigator_pref_tabbed_browsing"
File a bug on a new help topic?

>new file mode 100644
This isn't a new file, it's a copy of pref-tabs.dtd with some lines deleted.
(pref-links.xul probably looks too different to pref-tabs.xul though.)

>+<!ENTITY links.label "Link Behaviour">
Needs to be US spelling, unfortunately :-(
Attachment #451589 - Flags: superreview?(neil) → superreview+
(In reply to comment #20)
> From: Firefox 3.0
> 
> mozilla/browser/components/preferences/tabs.js
> 
> * browser.link.open_external
> * - determines where pages opened by external applications are opened:
> *     0 opens such links in the default window,
> *     1 opens such links in the most recent window or tab,
> *     2 opens such links in a new window,
> *     3 opens such links in a new tab
Actually 0 makes zero sense here, as this is the pref used when the API is passed a zero value. 1 is the default default ;-)
(Assignee)

Comment 26

7 years ago
Created attachment 453962 [details]
[for checkin] Patch v1.2 r=IanN sr=Neil

> (From update of attachment 451589 [details] [diff] [review])
> >-            
> >+
> Please try to avoid random whitespace changes. This one is OK though.
Fixed.

> >-   </prefpane>
> >+  </prefpane>
> As is this.
Fixed.

> >+                    helpTopic="navigator_pref_tabbed_browsing"
> File a bug on a new help topic?
Will do.

> >new file mode 100644
> This isn't a new file, it's a copy of pref-tabs.dtd with some lines deleted.
> (pref-links.xul probably looks too different to pref-tabs.xul though.)
Now using hg cp

> >+<!ENTITY links.label "Link Behaviour">
> Needs to be US spelling, unfortunately :-(
Fixed.
Attachment #451589 - Attachment is obsolete: true
Attachment #453962 - Flags: superreview+
Attachment #453962 - Flags: review+
(Assignee)

Comment 27

7 years ago
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/9fb2b78003f3
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 28

7 years ago
Help changes are needed: here or in another bug?
(Assignee)

Updated

7 years ago
Depends on: 574609

Updated

7 years ago
Target Milestone: --- → seamonkey2.1a3
You need to log in before you can comment on or make changes to this bug.