583625 - Consider adding UI for pref browser.link.open_newwindow.restriction

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, enhancement)

Description

[Jens Hatlak (:InvisibleSmiley)]

Bug 505311 changed the default of browser.link.open_newwindow.restriction from 0 to 2. Maybe this deserves its own preferences UI, e.g. on the new Link Behavior pane.

Cf. http://kb.mozillazine.org/Browser.link.open_newwindow.restriction

Comment 1

[Philip Chee]

Browser -> Link Behaviour -> "Link open behaviour" group box. Possibly a menulist at the bottom of this groupbox with three choices corresponding to browser.link.open_newwindow.restriction

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Looking at the possible values, we might actually want to use two widgets:
1. A checkbox at the top of the groupbox that disables the whole contents of the groupbox (checked: pref = 1, unchecked: pref != 1).
2. A checkbox at the bottom of the groupbox that enables the exception for windows that specify how they should be displayed (checked: pref = 2, unchecked: pref = 0 unless first checkbox is checked)

That's a bit more complicated wrt. the implementation but possibly cleaner in the UI than a drop-down that might come naturally from a programmer's POV but actually be harder for someone to come up with precise descriptions for the individual options.

Comment 3

[Philip Chee]

Unnecessary. We can use onsynctopref to trigger enabling/disabling the checkboxes in the groupbox. We do the same in some of our other panels. The menulist could be on top though.

Comment 4

[[e-mail]]

Attachment: Possible patch

Let the bikeshedding begin!

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #4)
> Created attachment 500552 [details] [diff] [review]
> Possible patch

So you disable open_newwindow.restriction UI when open_newwindow is 2. Did you come to this logic through code inspection? The MZ KB suggests that in fact open_newwindow should be disabled if open_newwindow.restriction is 1.

Pro Tip: Use attachment flags to get things reviewed or feedback. :-P

P.S. I wonder if the radiogroups should be indented (more). At least on Linux it looks all mixed now (only the bold headings are really distinguishable from the other text on the pane). Cf. the Downloads pane for what I'd recommend (although it's a checkbox instead of a description there).

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 500552 [details] [diff] [review]
Possible patch

Requesting review on Neil's behalf (which I'm only doing as he didn't assign himself either).

Comment 7

[Ian Neal]

Comment on attachment 500552 [details] [diff] [review]
Possible patch

if browser.link.open_newwindow = 2 on opening the links pref pane it does not disable the restrictionGroup only if you switch to 2 whilst the pane is open.
r-

Comment 8

[[e-mail]]

(In reply to comment #5)
> So you disable open_newwindow.restriction UI when open_newwindow is 2. Did you
> come to this logic through code inspection? The MZ KB suggests that in fact
> open_newwindow should be disabled if open_newwindow.restriction is 1.
There are three things that can be diverted into tabs:
1. <a target>
2. window.open without features
3. window.open with features
Gecko versions 1.8.1 through 1.9.2 did not distinguish between cases 1. and 2.
As a result, setting open_newwindow to 2 and setting restriction to 1 both had the same effect of turning off the diverts. However, prior to 1.8.1, and newly restored in Gecko 2, setting restriction to 1 does not affect <a target> which still diverts according to the open_newwindow preference.

(In reply to comment #7)
> if browser.link.open_newwindow = 2 on opening the links pref pane it does not
> disable the restrictionGroup only if you switch to 2 whilst the pane is open.
As Ratty pointed out I should have used onsynctopreference but for the purposes of this draft I was too lazy to add the necessary script file.

Comment 9

[[e-mail]]

Attachment: Revised patch

OK, let's see how you like this version.

I changed my mind about onsynctopreference; we seem to use Startup and onchange except when we want to manipulate the returned value.

Comment 10

[[e-mail]]

Pushed changeset 2d936fdce609 to comm-central.