Last Comment Bug 509664 - Restore browser.link.open_external hidden pref (as browser.link.open_newwindow.override.external)
: Restore browser.link.open_external hidden pref (as browser.link.open_newwindo...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: unspecified
: All All
: -- normal with 33 votes (vote)
: Firefox 10
Assigned To: Martin von Gagern
:
:
Mentors:
Depends on:
Blocks: cuts-control
  Show dependency treegraph
 
Reported: 2009-08-11 00:03 PDT by Martin von Gagern
Modified: 2013-09-09 09:18 PDT (History)
26 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (2.59 KB, patch)
2011-09-27 02:00 PDT, Martin von Gagern
dao+bmo: review-
Details | Diff | Splinter Review
Patch v2 (3.74 KB, patch)
2011-09-27 16:50 PDT, Martin von Gagern
no flags Details | Diff | Splinter Review
Patch v3 (4.09 KB, patch)
2011-09-28 05:59 PDT, Martin von Gagern
no flags Details | Diff | Splinter Review
Patch v4 (3.79 KB, patch)
2011-09-28 10:53 PDT, Martin von Gagern
dao+bmo: review+
Details | Diff | Splinter Review

Description Martin von Gagern 2009-08-11 00:03:46 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090810 Gentoo Firefox/3.5.2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.2) Gecko/20090810 Gentoo Firefox/3.5.2

Bug #324164 removed the distinction between opening a link with target="_blank" and some JavaScript open calls (preference browser.link.open_newwindow) and an URL opened by an external application (preference browser.link.open_external).

This change has made a lot of people unhappy, judging from the comments in bug #324164 as well as several bug reports (bug #469082, bug #477746 and their dups) for the specific case of open_newwindow=1, i.e. opening links in the same tab instead of a new tab or window.

I'm opening this bug here to give people who simply want the old behaviour back a place to vote. Those who read this and agree, please do vote!

Also there is little input from developers to the discussion on bug #324164, so it seems likely that devs aren't even listening, as bug #324164 comment #55 claims. Maybe a new bug report will get better attention than an ongoing discussion on a closed one.

As for the issue addressed by bug #324164: bug #324164 comment #36 does propose a solution to help avoid those issues without loosing functionality. It has been completely ignored. So before this report here gets closed as WONTFIX, some discussion as to why removing the functionality is necessary should take place, or a pointer to such a discussion in some other place should be posted here.

Reproducible: Always

Steps to Reproduce:
1. Click link in one tab
2. Run "firefox http://bugzilla.mozilla.org/"

Actual Results:  
Both cases treated the same, depending on open_newwindow.

Expected Results:  
Cases treated differently if the user configured them to behave differently.
E.g. open internal links in new tab, external in new window.
Or open internal links in same tab, external in new tab.
Comment 1 Aaron Adams 2009-08-11 08:40:43 PDT
It shouldn't even BE possible for an external link to replace a currently-opened page. The browser should assume if a page is currently open, the end user wants it to be open, not that the end user wants it to disappear. This seems dreadfully obvious.

There's NO WAY I should need to install an extension to handle this scenario. I'm a Web developer; I don't install extensions, period. I need to know the Web browser is always working as intended, unaffected by add-ons.

I'm still blown away that this change was made intentionally, and it remains the sole reason I'm not upgrading to Firefox 3.5.
Comment 2 Martin von Gagern 2009-08-11 09:04:01 PDT
(In reply to comment #1)
> It shouldn't even BE possible for an external link to replace a
> currently-opened page. The browser should assume if a page is currently open,
> the end user wants it to be open, not that the end user wants it to disappear.
> This seems dreadfully obvious.

Not necessarily. Closing and restarting FF can take its time. So there is a reason to leave pages open even if you want them replaced. One possible application for opening external links in the same tab might be the following: Consider some application where you regularly open web pages, e.g. for documentation. You'd probably type some code, request some documentation, read the documentation, and continue writing more code, all of this repeatedly. Having to close outdated tabs costs additional mouse clicks. If this kind of work is what you do all day, you might have a good reason to configure external apps to be opened in the same tab.

I agree that this is a rare scenario. But given the choice, I'd vote for an option that gives maximum freedom to the user, at least the experienced user (i.e. about:config but maybe no UI for this). I want the right to shoot myself in the foot.

> I'm still blown away that this change was made intentionally

I guess the change to unify stuff was intentional, but without sufficient consideration to the "open in same tab" case. I guess bug #469082 is most likely to deal with that specific case, but I'm not sure they'll do it by reintroducing the full flexibility of the old code.

(In reply to comment #0)
> As for the issue addressed by bug #324164: bug #324164 comment #36 does
> propose a solution to help avoid those issues without loosing functionality.

I just found that bug #469082 comment #8 has a very similar solution to offer, in a nice patch with only three lines of real code. On the whole that solution seems cleaner to me, as it does declare its preference and give a sane default.
Comment 3 Aaron Adams 2009-08-11 09:22:03 PDT
I'm all for flexibility, for sure. The more options for the power user, the better. But by default, I still think the browser should see an external request to open a page as an implicit request to ADD that page to the set of currently-opened pages.

In other words, the only entity that should be capable of replacing or destroying an open tab is that tab itself (and, by inheritance, the window to which it belongs).
Comment 4 Martin von Gagern 2009-08-11 10:04:14 PDT
(In reply to comment #3)
> But by default, I still think the browser should see an external
> request to open a page as an implicit request to ADD that page to the set of
> currently-opened pages.

I take it that this is the default for new profiles, and as there is no UI to change to the "open in same tab" preference, there is no way novice users can accidentialy stumble into this trap. The only exception to this is by legacy preferences from old versions of FF, where there was a distinction and an UI.

But I agree, it would make sense to ignore this legacy and instead force power users to redo their configuration if they really want it. I discussed a possible solution to this in bug #469082 comment #14.
Comment 5 RNicoletto 2009-08-13 01:07:12 PDT
(In reply to comment #0)
> 
> I'm opening this bug here to give people who simply want the old behaviour back
> a place to vote. Those who read this and agree, please do vote!

This is unnecessary. Since there are already open bugs about this argument, people should comment and vote on them and not here.

*** This bug has been marked as a duplicate of bug 469082 ***
Comment 6 paul 2009-09-08 16:11:53 PDT
I made an add-on to fix this bug:

https://addons.mozilla.org/en-US/firefox/addon/13626
Comment 7 Martin von Gagern 2009-09-10 13:34:46 PDT
The bug this here was duped to has been closed WONTFIX
by bug #469082 comment #19:
> [...] that's a different bug than this one, which is claiming that we're
> behaving incorrectly.

OK, so if that bug isn't a suitable request for a reintroduction of the features lost, then this one here is my second best bet.

I'm not claiming that current behaviour is incorrect according to any specs or similar, I only claim that it looses useful functionality, as outlined above and reiterated very eloquently by comments like bug #469082 comment #24.

REOPENING.
Comment 8 paul 2009-09-10 14:47:54 PDT
I agree, this bug is not a duplicate and makes a reasonable request.

The argument that it would "force" power users to re-do their config is weak. It seems reasonable to assume that anyone who set that hidden preference in the first place did so because they wanted it's functionality, which has now been taken away from them.

Besides, it would be trivial to re-introduce the preference with a slightly different name.
Comment 9 Alex Limi (:limi) — Firefox UX Team 2010-05-12 18:58:54 PDT
Indeed, external links should never replace open tabs. This has hereby been added to the Paper Cuts project. :)
Comment 10 Aaron Adams 2010-05-12 20:00:59 PDT
While I've since switched to Chrome for day-to-day browsing and development, the fact you now HAVE a "Paper Cuts" project tells me Firefox is (hopefully) back on the right track, and is (hopefully) no longer putting Firefox developers ahead of Firefox users, nor consistency ahead of common sense. Kudos to you, Alexander and the rest! I'm really glad to see this.
Comment 11 Martin von Gagern 2010-05-13 03:05:49 PDT
(In reply to comment #9)
> Indeed, external links should never replace open tabs. This has hereby been
> added to the Paper Cuts project. :)

This bug here is about restoring the old open_external functionality. In particular, it's about the power to configure behaviour for external and internal links independently, preferrably including the possibility of external links replacing open tabs if the user wants it this way.

Firefox replacing open tabs due to legacy configs is probably best addressed in bug #469082. Don't know about the Paper Cuts project, and couldn't find a link quickly, but the name and its Ubuntu definition [1] sounds like it would apply to unexpected behaviour which novice users might accidentially stumble upon.

This bug here was intended with a different audience in mind: it's not about a novice user and what he might accidentially have configured, but instead it's about the power user who could configure certain behaviour in the past, and cannot achieve the same behaviour in recent versions, thus limiting his power and causing a regression.

As argued in comment #2, there might be use cases where external links should replace open tabs. If it's easy to achieve, and if it only happens when users deliberately configure it this way, e.g. via about:config, then why forbid it? And deliberate configuration could e.g. be ensured by using a new config name, and not exposing it in the configuration dialog UI.

I appreciate your interest in this bug, but as there are a lot of bugs about external links replacing open tabs, please keep the wider scope of this bug here in mind, and don't turn it into yet another one about that narrow issue.

[1] https://wiki.ubuntu.com/PaperCut
Comment 12 BoffinbraiN 2010-07-17 16:44:12 PDT
> In other words, the only entity that should be capable of replacing or
> destroying an open tab is that tab itself (and, by inheritance, the window to
> which it belongs).

Absolutely.  This sort-of reminds me of programmers re-using mutable objects because of the perceived overhead involved in creating new ones, without thinking about the consequences.

I can't see how anyone could argue that this behaviour is not the correct one.  Let's keep voting.
Comment 13 william.w.cline 2010-08-31 17:07:26 PDT
+1. I, too, want to configure external hyperlinks’ behavior separately.
Comment 14 generaleisen 2011-06-18 11:21:49 PDT
+1 Please bring this option back.
Comment 15 Sergey Galanov 2011-09-08 11:40:42 PDT
This is absolutely ridiculous. Please bring the old behavior back.
Comment 16 BoffinbraiN 2011-09-08 12:09:52 PDT
I had to disable Tab Mix Plus today because of some conflict in Firefox 6.0.2 that caused the middle mouse button to stop opening new tabs.  Anyway, the point is that as soon as I disabled it, I remembered this bug.

The ONLY reason I have such a heavyweight addon installed, and probably suffer any associated performance costs, is because of how much I hate THIS bug.

So, I guess I can say my excuse for posting this is to acknowledge that it is still present in 6.0.2.
Comment 17 Thomas Seidl 2011-09-08 14:40:24 PDT
Still there in 6.x? This bug is becoming more and more of a disgrace for Firefox …
Please fix this!
Comment 18 Aaron 2011-09-26 19:34:55 PDT
I agree for the need for this option. I have a separate window created for a couple special tasks (no longer willing to use Prism or modern versions of it because they're too bloated), and do NOT want new tabs added to those windows!
Comment 19 Aaron 2011-09-26 19:42:40 PDT
Ideally, we would have the option to set which FF window will receive links launched from external programs. Then can keep special-task windows pristine, but NOT open a whole bunch of tabs if a lot being sent from other programs!
Comment 20 Alex Limi (:limi) — Firefox UX Team 2011-09-26 22:42:32 PDT
No need to add more comments until this has been resolved. We consider this a bug, and it should be fixed.

Links from external programs should never replace tabs, nor open new windows. They should open as a new tab in the most recently active window that is a window with tabs (e.g. not a pop-up window).
Comment 21 Martin von Gagern 2011-09-27 00:19:42 PDT
(Quoting myself from comment #0)
[...]
> I'm opening this bug here to give people who simply want the old behaviour
> back a place to vote.
[...]
> Expected Results:  
> Cases treated differently if the user configured them to behave differently.
> E.g. open internal links in new tab, external in new window.

(In reply to Alex Limi (:limi) — Firefox UX Team from comment #20)
> Links from external programs should never replace tabs, nor open new
> windows.

That is not what this bug report asks for. This bug report was opened with a single goal: restore the flexibility of configuration available before the "fix" for bug #324164 was included. With open_external, one had:
- two distinct options, one for internal and one for external links
- three values for each: same tab, new tab, new window
That flexibility might be restricted to direct about:config manipulation, i.e. no UI required, and novice users won't be confused by this.

So I very much disagree with the statement "Links from external programs should never [...] open new windows", as that is just the opposite of what I wanted when filing this report here. Any discussion about ideal DEFAULT behaviour is a different thing, and doesn't belong on this report here.

Notice that I already told Alex Limi in comment #11 that his interpretation of this bug report here doesn't match the intent of the original report.

(In reply to Aaron from comment #19)
> Ideally, we would have the option to set which FF window will receive links
> launched from external programs.

Any ideas about other aproaches please file separate feature requests or use http://input.mozilla.org/idea/ to describe your ideas.

As this comment here turns into a summary of previous comments, please note that comment #2 points to an existing patch at bug #469082 comment #8 to address the issue this bug here is all about. Do I have to attach it here for review?

Please keep this bug focused on one single issue only: giving power users all the flexibility that the old open_external offered, no more and no less. Anything else is a different bug, feature request, idea, or whatever.
Comment 22 Alex Limi (:limi) — Firefox UX Team 2011-09-27 00:49:25 PDT
Then I recommend this bug be marked as invalid, since that's not the behavior we're going for. Sensible defaults in the core, use add-ons to deviate from the behavior common users need and expect.
Comment 23 Martin von Gagern 2011-09-27 01:39:14 PDT
(In reply to Alex Limi (:limi) — Firefox UX Team from comment #22)
> Then I recommend this bug be marked as invalid, since that's not the
> behavior we're going for.

That doesn't make the request INVALID. There used to be existing functionality, and people were using it, for very different reasons. Then some release took away that functionality, thus causing a loss of features. That's the reason this was filed as a bug, not a feature request. I still believe that view to be valid.

If you are absolutely set on not restoring that functionality, then the proper status would be "WONTFIX". The request itself is valid, but you might choose not to address it nevertheless.

> Sensible defaults in the core, use add-ons to
> deviate from the behavior common users need and expect.

As I fail to see any drawbacks from mozilla committing the three-line patch from bug #469082 comment #8, I'm not happy with this statement. I can understand you not committing changes that would introduce adverse effects for "common users", or not coding patches yourself unless they benefit those "common users". But the patch is there, I can see no problems at all with it, so why needlessly narrow down the set of users you support? Why not simply review those three lines?
Comment 24 Martin von Gagern 2011-09-27 02:00:00 PDT
Created attachment 562685 [details] [diff] [review]
Proposed patch

OK, here is the patch, mostly as Jesper Kristensen wrote it for bug #469082 comment #8, but tweaked for line breaks and recent mozilla-central code changes. Will you review this?
Comment 25 Dão Gottwald [:dao] 2011-09-27 05:27:57 PDT
Comment on attachment 562685 [details] [diff] [review]
Proposed patch

>+    if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW && isExternal)
>+      aWhere = gPrefService.getIntPref("browser.link.open_newwindow.override.external");
>     if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW)
>       aWhere = gPrefService.getIntPref("browser.link.open_newwindow");

This looks wrong... aWhere as set by your added lines wouldn't stick, the existing code would always overwrite it.
Comment 26 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-09-27 08:50:57 PDT
I don't think we want to re-add the pref.

We want comment 20 fixed (if it's still an issue), and that doesn't involve adding a pref. If this bug is about just adding a pref, I think it's WONTFIX.
Comment 27 Dão Gottwald [:dao] 2011-09-27 11:14:26 PDT
A hidden override pref with no default value would be no significant maintenance burden, so I don't see why we wouldn't support that.

Comment 20 is based on a misunderstanding.
Comment 28 Martin von Gagern 2011-09-27 14:17:15 PDT
(In reply to Dão Gottwald [:dao] from comment #25)
> This looks wrong... aWhere as set by your added lines wouldn't stick, the
> existing code would always overwrite it.

First off: I patched my own FF6 here (I love Gentoo!), and the patch works as advertised. So I guess you've made a mistake reading the patch.

I also cannot understand why you think the setting would always be overwritten. OPEN_DEFAULTWINDOW is an alias for 0. So if aWhere is unset and the resource is external, then set it to the override config value. If the value is still unset, either because the resource was not external or because the config was at its default value, then use the newwindow config value.

In other words, if the first "if" suceeded and the override config is nonzero, then the second "if" will not match, thus will not overwrite the override setting.

Dão, please let me know if this explanation was any more understandable than the actual code, or if not, what makes you still believe that the setting would be overwritten when in fact it is not overwritten.
Comment 29 Dão Gottwald [:dao] 2011-09-27 15:17:44 PDT
I see... The 0 case is somewhat confusing, since you're not overriding browser.link.open_newwindow that case.
Adding to the confusion is that 0 is documented as a valid value for browser.link.open_newwindow. AFAIK the whole point of the pref is to define the behavior for OPEN_DEFAULTWINDOW (0), so the only meaningful options are 1, 2 and 3.
Comment 30 Martin von Gagern 2011-09-27 15:41:27 PDT
(In reply to Dão Gottwald [:dao] from comment #29)
> I see... The 0 case is somewhat confusing, since you're not overriding
> browser.link.open_newwindow that case.

Slightly confusing, yes, due to the order, because the override precedes the thing it overrides. But I doubt you'd want to bloat it to the following alternative, which I personally find even harder to read:

+    aWhereWasSet = (aWhere != Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW);
     if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW)
       aWhere = gPrefService.getIntPref("browser.link.open_newwindow");
+    override =
+      gPrefService.getIntPref("browser.link.open_newwindow.override.external");
+    if (!aWhereWasSet && override != 0 && isExternal)
+      aWhere = override
     switch (aWhere) {

As the current code processes config settings in decreasing order of priority, from command line to user config, keeping that order requires inserting the override between the command line and the user config, the way the patch does it.

> Adding to the confusion is that 0 is documented as a valid value for
> browser.link.open_newwindow. AFAIK the whole point of the pref is to define
> the behavior for OPEN_DEFAULTWINDOW (0), so the only meaningful options are
> 1, 2 and 3.

Agreed, if by "documented" you mean the comment in the source code. User-visible documentation at http://kb.mozillazine.org/About:config_entries doesn't include 0 as a possible value. But in any case, that documentation is not really the issue here. Or do you want me to include a tweak for the comment in my patch?

Will you change the review status for the patch, now that this is clarified?
Comment 31 Dão Gottwald [:dao] 2011-09-27 15:48:43 PDT
(In reply to Martin von Gagern from comment #30)
> (In reply to Dão Gottwald [:dao] from comment #29)
> > I see... The 0 case is somewhat confusing, since you're not overriding
> > browser.link.open_newwindow that case.
> 
> Slightly confusing, yes, due to the order, because the override precedes the
> thing it overrides. But I doubt you'd want to bloat it to the following
> alternative, which I personally find even harder to read:
[..]

No, that doesn't seem better. I think I'd prefer if the override pref by default was absent or set to something (e.g. -1) that can't be confused with a value defined by nsIBrowserDOMWindow.

> Agreed, if by "documented" you mean the comment in the source code.
> User-visible documentation at http://kb.mozillazine.org/About:config_entries
> doesn't include 0 as a possible value. But in any case, that documentation
> is not really the issue here. Or do you want me to include a tweak for the
> comment in my patch?

Yeah, I think the comment should be fixed. There's a similar one in browser/components/preferences/tabs.js.
Comment 32 Martin von Gagern 2011-09-27 16:50:12 PDT
Created attachment 562910 [details] [diff] [review]
Patch v2

(In reply to Dão Gottwald [:dao] from comment #31)
> I think I'd prefer if the override pref by
> default was absent or set to something (e.g. -1) that can't be confused with
> a value defined by nsIBrowserDOMWindow.
[...]
> Yeah, I think the comment should be fixed. There's a similar one in
> browser/components/preferences/tabs.js.

OK, adjusted the patch. I personally find that changing the default from 0 to -1 doesn't really help the situation, as people now might wonder where that -1 came from, and whether there was a particular reason not to use 0 as the default. After all, the meaning of 0 as "use the default" matches the override.external semantics rather well. So I still prefer the first patch.
Comment 33 Dão Gottwald [:dao] 2011-09-27 17:25:12 PDT
Comment on attachment 562910 [details] [diff] [review]
Patch v2

> // handle links targeting new windows
>-// 0=default window, 1=current window/tab, 2=new window, 3=new tab in most recent window
>+// 1=current window/tab, 2=new window, 3=new tab in most recent window
> pref("browser.link.open_newwindow", 3);
> 
>+// handle external links (i.e. links opened from a different application)
>+// -1: follow browser.link.open_newwindow
>+// 1: current window/tab
>+// 2: new window
>+// 3: new tab in most recent window
>+pref("browser.link.open_newwindow.override.external", -1);

You can just refer to browser.link.open_newwindow for values != -1.

>-    if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW)
>+    if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW && isExternal)
>+      aWhere = gPrefService.getIntPref("browser.link.open_newwindow.override.external");
>+    if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW || aWhere == -1)
>       aWhere = gPrefService.getIntPref("browser.link.open_newwindow");

aWhere shouldn't be set to -1, not even temporarily. Here's how I think this should look like:

if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW) {
  if (isExternal &&
      gPrefService.prefHasUserValue("browser.link.open_newwindow.override.external"))
    aWhere = gPrefService.getIntPref("browser.link.open_newwindow.override.external");
  else
    aWhere = gPrefService.getIntPref("browser.link.open_newwindow");
}

or maybe:

if (aWhere == Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW) {
  aWhere = gPrefService.getIntPref(
             isExternal &&
             gPrefService.prefHasUserValue("browser.link.open_newwindow.override.external") ?
               "browser.link.open_newwindow.override.external" :
               "browser.link.open_newwindow");
}
Comment 34 Aaron 2011-09-27 21:41:13 PDT
(In reply to Aaron Adams from comment #3)
> ... by default, I still think the browser should see an external
> request to open a page as an implicit request to ADD that page to the set of
> currently-opened pages.

I think this could be fine IF we can designate certain windows NOT to receive links from other programs (that would preserve the ability to keep certain windows dedicated to certain functions!)...
Comment 35 Martin von Gagern 2011-09-28 05:46:22 PDT
(In reply to Aaron from comment #34)
> I think this could be fine IF we can designate certain windows NOT to
> receive links from other programs (that would preserve the ability to keep
> certain windows dedicated to certain functions!)...

I don't see how this relates to the issue at hand. If I understand you right, you want to add power to the existing "open in new tab" functionality, to prevent those tabs to be opened in particular windows. That's all very fine, but not what this bug report here is asking for, so please file a different report for this.
Comment 36 Martin von Gagern 2011-09-28 05:59:12 PDT
Created attachment 563043 [details] [diff] [review]
Patch v3

(In reply to Dão Gottwald [:dao] from comment #33)
> You can just refer to browser.link.open_newwindow for values != -1.

OK, did so.

> aWhere shouldn't be set to -1, not even temporarily. Here's how I think this
> should look like:

So be it.
Comment 37 Dão Gottwald [:dao] 2011-09-28 08:39:53 PDT
Comment on attachment 563043 [details] [diff] [review]
Patch v3

>+      if (isExternal && gPrefService.prefHasUserValue("browser.link.open_newwindow.override.external"))

I intentionally broke the line after &&, as this line is overly long...
Comment 38 Martin von Gagern 2011-09-28 10:53:05 PDT
Created attachment 563112 [details] [diff] [review]
Patch v4

(In reply to Dão Gottwald [:dao] from comment #37)
> I intentionally broke the line after &&, as this line is overly long...

OK, makes sense. Bugzilla had mostly garbled you original formatting.

Still long lines at 90 characters, but with the length of the option, that is difficult to avoid. And judging from the surrounding code, breaking the line after the method name but before the opening parenthesis of the argument list doesn't agree with mozilla's style, does it?
Comment 39 Dão Gottwald [:dao] 2011-10-02 12:19:39 PDT
Comment on attachment 563112 [details] [diff] [review]
Patch v4

Looks good to me. I won't land this immediately so that Gavin has a chance to speak up if he still thinks we shouldn't take this patch.
Comment 40 Dão Gottwald [:dao] 2011-10-02 12:22:42 PDT
(In reply to Martin von Gagern from comment #38)
> Still long lines at 90 characters, but with the length of the option, that
> is difficult to avoid. And judging from the surrounding code, breaking the
> line after the method name but before the opening parenthesis of the
> argument list doesn't agree with mozilla's style, does it?

I don't think it does.
90 characters also aren't really a problem. We're not very strict about the exact limit these days.
Comment 42 :Ms2ger (⌚ UTC+1/+2) 2011-10-09 07:35:16 PDT
https://hg.mozilla.org/mozilla-central/rev/d4efc5ab5877
Comment 43 Nicolas Barbulesco 2013-09-07 15:55:30 PDT
(In reply to Alex Limi (:limi) — Firefox UX Team from comment #20)

> We consider this
> a bug, and it should be fixed.

Too bad this is still not fixed.

> Links from external programs should never replace tabs, nor open new
> windows. They should open as a new tab in the most recently active window
> that is a window with tabs (e.g. not a pop-up window).

I disagree. The most recently active Firefox window has nothing to do with the link coming from Windows’ help. There are users like me that don't want external links to mess with existing tabs, but who want them to open in new windows.
Comment 44 Wojtek Lerch 2013-09-07 21:35:18 PDT
Unless I have declared that I never want to have more than one window, links from external programs should definitely *not* add tabs to unrelated browser windows.  Why do people want to be able to have multiple browser windows with multiple tabs in them -- isn't it to keep related content in the same window, and to keep unrelated content in separate windows?  PLEASE DON'T MAKE THAT IMPOSSIBLE.  When I click on something in a window that opens a new tab, I want the new tab to be in the window I clicked on, because it's related.  When I click on something in an external program, it's impossible to open a browser tab in the same window; but the next closest thing is not to hijack one of the unrelated windows that I am not currently working with, but to open a new window.

Ideally, it would be nice if the browser remembered that this new window is related to my external program, and added tabs to it when I click on more things in *the same* external program; but if that's impossible or too hard, opening a new window every time is much better than arbitrarily picking one of my unrelated windows and adding a new tab to it.  (It's even more annoying when a minimized window is hijacked -- neither unminimizing it nor leaving it minimized is a good choice when you add a new tab to it.)
Comment 45 Martin von Gagern 2013-09-08 11:24:48 PDT
(In reply to Nicolas Barbulesco from comment #43)
> There are users like me that […] want external links […] to open in new windows.

Setting browser.link.open_newwindow.override.external to 2 should do this these days.

(In reply to Wojtek Lerch from comment #44)
> PLEASE DON'T MAKE THAT IMPOSSIBLE.

It is possible again, since this bug here got fixed. The feature is pretty hidden, you need to use about:config to change that setting, but the feature has been reestablished.

> Ideally, it would be nice if the browser remembered that this new window is
> related to my external program, and added tabs to it when I click on more
> things in *the same* external program;

Nice idea, might require help from the apps. In any case it's beyond the scope of this bug here. Feel free to post a separate request for enhancement.
Comment 46 Wojtek Lerch 2013-09-08 21:14:05 PDT
(In reply to Martin von Gagern from comment #45)
> Setting browser.link.open_newwindow.override.external to 2 should do this
> these days.

Indeed -- I guess I can uninstall the Restore Open_External extension now...  Thanks.
Comment 47 Nicolas Barbulesco 2013-09-09 08:28:51 PDT
(In reply to Wojtek Lerch from comment #44)

> Unless I have declared that I never want to have more than one window, links
> from external programs should definitely *not* add tabs to unrelated browser
> windows.  Why do people want to be able to have multiple browser windows
> with multiple tabs in them -- isn't it to keep related content in the same
> window, and to keep unrelated content in separate windows?  PLEASE DON'T
> MAKE THAT IMPOSSIBLE.  When I click on something in a window that opens a
> new tab, I want the new tab to be in the window I clicked on, because it's
> related.  When I click on something in an external program, it's impossible
> to open a browser tab in the same window; but the next closest thing is not
> to hijack one of the unrelated windows that I am not currently working with,
> but to open a new window.

> but if that's impossible or too hard,
> opening a new window every time is much better than arbitrarily picking one
> of my unrelated windows and adding a new tab to it.  (It's even more
> annoying when a minimized window is hijacked -- neither unminimizing it nor
> leaving it minimized is a good choice when you add a new tab to it.)

I agree with the good sense of Wojtek.

(In reply to Martin von Gagern from comment #45)

> > There are users like me that […] want external links […] to open in new windows.
> 
> Setting browser.link.open_newwindow.override.external to 2 should do this
> these days.

Except that this hidden pref is not documented, not even in the page kb.mozillazine.org/About:config_entries .
Comment 48 Martin von Gagern 2013-09-09 09:18:29 PDT
(In reply to Nicolas Barbulesco from comment #47)
> Except that this hidden pref is not documented, not even in the page
> kb.mozillazine.org/About:config_entries .

“MozillaZine is not run by or formally associated with Mozilla”, as their page informs us, plus it's a wiki. So feel free to create an account and document this thing. Or report it at http://forums.mozillazine.org/viewtopic.php?f=11&t=2734325.

I agree that this should be documented, but I try to keep this issue here focused, even after it got fixed. If it ends with a post in the spirit of “not completely fixed yet”, then others might miss the existing implementation for browser.link.open_newwindow.override.external. Just as you apparently did.

Note You need to log in before you can comment on or make changes to this bug.