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

RESOLVED FIXED in seamonkey2.1b2

Status

SeaMonkey
Preferences
--
enhancement
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: InvisibleSmiley, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
seamonkey2.1b2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

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

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

Updated

7 years ago
Whiteboard: [good first bug]
(Reporter)

Comment 2

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

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

Comment 4

7 years ago
Created attachment 500552 [details] [diff] [review]
Possible patch

Let the bikeshedding begin!
(Reporter)

Comment 5

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

Updated

7 years ago
Blocks: 78037
(Reporter)

Comment 6

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

Updated

7 years ago
Assignee: nobody → neil
Status: NEW → ASSIGNED

Comment 7

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

Comment 8

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

Comment 9

7 years ago
Created attachment 508772 [details] [diff] [review]
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.
Attachment #500552 - Attachment is obsolete: true
Attachment #508772 - Flags: review?(iann_bugzilla)

Updated

7 years ago
Attachment #508772 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 10

7 years ago
Pushed changeset 2d936fdce609 to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug]

Updated

7 years ago
Target Milestone: --- → seamonkey2.1b2
(Reporter)

Updated

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