Note: There are a few cases of duplicates in user autocompletion which are being worked on.

preference browser.link.restriction=1 is essentially useless

RESOLVED FIXED

Status

SeaMonkey
UI Design
--
minor
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Benoît, Assigned: neil@parkwaycc.co.uk)

Tracking

({regression})

1.8 Branch
x86
All
regression
Bug Flags:
blocking-seamonkey1.1b -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
This setting is related to the preferences that deal with how to open a link that calls for a new window.

On the 1.8.0 branch, setting this preference
-to 0 diverts everything
-to 1 diverts everything but window.open (JavaScript)
-to 2 diverts everything but window.open with attributes like width and height

Now, on the 1.8.1 branch, setting this preference
-to 0 diverts everything
-to 1 diverts nothing
-to 2 diverts everything but window.open with attributes

So option 1 got changed, and is now basically useless.

Updated

11 years ago
Flags: blocking-seamonkey1.1b?
(Assignee)

Comment 1

11 years ago
To clarify, in 1.8.0 a restriction of 1 still diverted target="_blank" according to the open_newwindow preference and you used to have to set that preference to 2 to make everything open in new windows.

Comment 2

11 years ago
Option "1" is basically the "ignore 'browser.link.open_external'" option.  That seems to have been the original intent; the fact that it didn't apply to <a target="_blank"> was a copy-paste error in the code as far as I could tell.
(Reporter)

Comment 3

11 years ago
Why's that even necessary? To ignore browser.link.open_external is essentially the same as setting that option to the default, which is to open links in new windows.

Comment 4

11 years ago
> Why's that even necessary?

You'll have to ask danm.  He wrote that code.

Is there UI for this pref?

Comment 6

11 years ago
No.
Flags: blocking-seamonkey1.1b? → blocking-seamonkey1.1b-

Updated

11 years ago
Keywords: relnote
(Reporter)

Comment 7

11 years ago
So, has a decision on this pref been reached yet? I talked to Neil about this a month ago, and he was going to talk to bz about it.
Filter "spam" on "guifeatures-nobody-20080610".
Assignee: guifeatures → nobody
QA Contact: guifeatures

Updated

9 years ago
Component: XP Apps: GUI Features → UI Design
(Assignee)

Comment 9

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

By passing aCalledFromJS down to nsContentTreeOwner we can make the restriction only apply for window.open as documented in firefox.js/browser-prefs.js, i.e.
0: target="target" and window.open(url, target, args) diverted
1: target="target" diverted
2: target="target" and window.open(url, target) diverted
Setting browser.link.open_newwindow to 2 still works to disable all diverts.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #452249 - Flags: review?(bzbarsky)

Comment 10

7 years ago
What happened to comment 2?
(Assignee)

Comment 11

7 years ago
(In reply to comment #2)
> Option "1" is basically the "ignore 'browser.link.open_external'" option.  That
> seems to have been the original intent; the fact that it didn't apply to <a
> target="_blank"> was a copy-paste error in the code as far as I could tell.
Paste where? Even the comment for where the restriction pref is read refers specifically to window.open, I don't see any restriction for target="blank".

Comment 12

7 years ago
> Paste where? 

In the original code that was checked in to handle the various window.open retargeting prefs by danm.

Comment 13

7 years ago
That is, the comments were written post-facto to "explain" what the code was doing, but the original code was trying to have the codepaths be the same and just failed.  At least if I remember things correctly; it's been a while since I did the archeology.
(Assignee)

Comment 14

7 years ago
Well, attachment 159442 [details] [diff] [review] seems to be the original patch that just diverted <a href="..." target="...">; attachment 159443 [details] [diff] [review] looks like it was designed as a window.open follow-up, with the restriction pref added as an afterthought, so that people could choose not to divert window.open only (given that the second option of only diverting without features had not yet been thought of).

Comment 15

7 years ago
OK, good.

I'm going to need a few days here to find time to sort through the callers and make sure the aCalledFromJS thing does what we want it to.

Comment 16

7 years ago
How does this interact with showModalDialog (which passes false for aCalledFromJS)?
(Assignee)

Comment 17

7 years ago
(In reply to comment #16)
> How does this interact with showModalDialog (which passes false for
> aCalledFromJS)?
showModalDialog never tries to Provide a window, because it's a dialog; not only are CHROME_MODAL and CHROME_DIALOG both set, but because showModalDialog always passes a real nsIVariant, even for null, then aDialog gets set too.

Comment 18

7 years ago
Comment on attachment 452249 [details] [diff] [review]
Possible patch

Sounds good.  Thanks for the explanation!
Attachment #452249 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 19

7 years ago
Pushed changeset a9d27938bb2d to mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/a9d27938bb2d
http://hg.mozilla.org/mozilla-central/rev/f469d7357b5b
(Assignee)

Comment 21

7 years ago
Comment on attachment 452249 [details] [diff] [review]
Possible patch

Oops. I forgot that this was a -w patch and forgot to recreate the original patch to get the indentation right.
Could this have cause bug 651659?
(Assignee)

Comment 23

6 years ago
It's only supposed to affect content windows opened using target attributes.
I figured as much, but maybe it's not working as expected. It seems that there weren't a lot of changes in this area from 1.9.2 to 2.0, so I'm not sure what else could be the culprit.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.