661120 - about dialog links should open in new tabs rather than new windows

Status: RESOLVED DUPLICATE of bug 263433

(Firefox :: General, defect)

Description

[baffclan]

Steps to Reproduce:
1. Start Firefox
2. Help -> About Nightly
3. apear about dialog
4. click "Licensing Information"


Actual Results:
about:license was opened in New Window

Expected Results:
about:license was opened in new tab current window

Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110530 Firefox/7.0a1

On Thunderbird, about:license was opened in new tab.

Comment 1

[Justin Dolske [:Dolske]]

Haven't looked at this code, but should be a simple fix... Call getMostRecentBrowserWindow() and open the tab there, with a fallback for opening in a new window in the unlikely case that there are no browser windows open.

Comment 2

[:Margaret Leibovic]

Attachment: patch

This patch does the job. I decided to also close the about window when one of these links is clicked; it seemed awkward to open a link in a tab but keep the about window open on top of that tab, especially on Windows, where the about window cannot be moved to the background.

Comment 3

[Justin Dolske [:Dolske]]

No fallback to opening a window for when getMostRecentBrowserWindow() returns null?

Comment 4

[:Margaret Leibovic]

Attachment: patch v2

Added fallback. I also added a line to focus topWindow if it exists, since loadOneTab doesn't do that for us.

I also noticed bug 263433, which would solve this problem more generally, but that seems like it isn't going anywhere any time soon, so this workaround is probably a good call. Perhaps we can file a follow-up to revert these changes once that bug is fixed.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

It would be ideal to be able to use openUILinkIn(url, "tab"), since that already handles all of this correctly (in addition to checking for popups etc.). I guess you could just include utilityOverlay.js to make that possible.

Comment 6

[:Margaret Leibovic]

Attachment: patch v3

Now using openUILinkIn.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 536994 [details] [diff] [review]
patch v3

The one thing I'm not sure about from testing this is whether closing the dialog makes sense - someone might want to click two links, and having to reopen the dialog to do that is a bit annoying. I suppose not closing it is problematic because it means you have to worry about focusing the right window again, which is hard to do if you're using openUILinkIn :(

(also needs trivial rebase on top of bug 659972)

Comment 8

[:Margaret Leibovic]

On Windows, you can't focus anything on top of the about dialog, so you wouldn't even be able to get into a situation of needing to focus the right window again. And on OSX, the about window isn't in the window list, so if it went behind a focused browser window, you would need to open it the same way again anyway. It seems like there's not really a good solution if we want the new tab to get focus.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Tried the patch again, and it still feels weird. I'm in the middle of reading the paragraph, click the first link, and then the window goes away and I'm on mozilla.org. If keeping the about dialog open was an option on Windows I'd suggest doing that, but given that it's not I'm leaning towards just not bothering with this and keeping the current new-window behavior - i.e. WONTFIX.

Comment 10

[:Margaret Leibovic]

It seems like the main issue that's causing this weirdness is that the about window is a modal dialog, but it looks more like a normal window, so you expect it to stay open as a normal window would. Multiple bugs seem to be caused by the fact that this is a dialog instead of a normal window. Bug 608934 already changed the dialog into a window on Linux, but it might be worth thinking about doing that for all platforms. I can investigate existing bugs on this issue, but bug 665285 is the one I saw recently that made me think about this.

Comment 11

[:Margaret Leibovic]

Comment on attachment 536994 [details] [diff] [review]
patch v3

I'm canceling review, since this seems like a sub-optimal solution.

Comment 13

[Thomas Ahlblom]

Mozilla/5.0 (X11; Linux x86_64; rv:8.0a1) Gecko/20110727 Firefox/8.0a1

Currently the platform of this bug is x86 / Windows XP - but isn't it for all platforms? The behavior seems to be the same on Linux.

Comment 14

[:Margaret Leibovic]

(In reply to Thomas Ahlblom from comment #13)
> Mozilla/5.0 (X11; Linux x86_64; rv:8.0a1) Gecko/20110727 Firefox/8.0a1
> 
> Currently the platform of this bug is x86 / Windows XP - but isn't it for
> all platforms? The behavior seems to be the same on Linux.

Yes, this does affect all platforms. However, we had a tough time coming up with an appropriate solution because the about window behavior is different on different platforms. I filed bug 686254 on trying that change that.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

I'll help mentor this bug. We should open the links in new tabs but not close the About dialog.

Comment 16

[u428464]

In-content "about" would solve the problem.

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Guillaume C. [:GE3K0S] from comment #16)
> In-content "about" would solve the problem.

See https://bugzilla.mozilla.org/show_bug.cgi?id=686254#c4 for explanation why we won't move the About window to in-content.

Comment 18

[Jon Rietveld]

Attachment: opens link in open window and keeps dialog box open

Unsure if it is better to change all openLink in aboutDialog.xul to openUILinkIn('url', 'tab'); or leave openLink() in aboutDialog.js that calls openUILinkIn

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 620378 [details] [diff] [review]
opens link in open window and keeps dialog box open

Hm... I don't think we can use onclick here because it would break keyboard access (which is actually already broken, see bug 751444).

If we did use onclick we would also have to use onkeydown as well. browser.js does this with the addLinkClickCallback function (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7072).

I guess we'll want to fix bug 751444 first before we fix this bug.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 620378 [details] [diff] [review]
opens link in open window and keeps dialog box open

Review of attachment 620378 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. I tested this on Windows, Linux, and Mac. It also works with keyboard navigation, so we won't need to do use the aforementioned addLinkClickCallback function.

Comment 21

[Dão Gottwald [:dao]]

Why is a tab competing for focus with the dialog better than a new window?

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Dão Gottwald [:dao] from comment #21)
> Why is a tab competing for focus with the dialog better than a new window?

Because we would rather users not have another window created, and we also build our browser around tab-browsing instead of window-browsing.

If we create a new window and the user then uses that window to begin browsing, their browsing session will be distributing among multiple windows, which can confuse them when they are looking for tabs from their original window.

Comment 23

[Dão Gottwald [:dao]]

(In reply to Jared Wein [:jaws] from comment #22)
> (In reply to Dão Gottwald [:dao] from comment #21)
> > Why is a tab competing for focus with the dialog better than a new window?
> 
> Because we would rather users not have another window created, and we also
> build our browser around tab-browsing instead of window-browsing.

This says no word about the downside I was hinting at. The dialog remains in the foreground and obscures the new tab, which is weird.

Comment 24

[Frank Yan (:fryn)]

(In reply to Dão Gottwald [:dao] from comment #23)
> (In reply to Jared Wein [:jaws] from comment #22)
> This says no word about the downside I was hinting at. The dialog remains in
> the foreground and obscures the new tab, which is weird.

I concur on this point. We should make the dialog not always-on-top. It's not a modal dialog, so we shouldn't mislead users into thinking it is. I was fooled by it until Jared told me today that it isn't. It already isn't always-on-top on Mac.

The real solution I recommend is to make aboutDialog.xul open in a tab itself first, then fix this bug. We shouldn't ever have users thinking that they need to wait around staring at the dialog, while it downloads an update.

Comment 25

[Frank Yan (:fryn)]

I realize that there was a bug filed to do just that, and it got wontfix'd, but I don't buy the arguments in that bug. Plus, we are already moving most, if not all, of our app-global UI that used to be windows into tabs.

Comment 26

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Makes the About dialog not top-most on Windows and Linux

This patch makes the About dialog not top-most on Windows and Linux. It also has the negative effect that if you close its opener the About dialog won't be closed (but maybe that is a good thing).

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 623853 [details] [diff] [review]
Makes the About dialog not top-most on Windows and Linux

Nevermind, I don't want to make it not top-most.

Comment 28

[Frank Yan (:fryn)]

(In reply to Jared Wein [:jaws] from comment #26)
(In reply to Jared Wein [:jaws] from comment #27)
> Nevermind, I don't want to make it not top-most.

As I wrote above, I think we should just make aboutDialog.xul open in a tab (in a separate bug blocking this one) and then fix this bug.

Comment 29

[u428464]

(In reply to Frank Yan (:fryn) from comment #25)
> I realize that there was a bug filed to do just that, and it got wontfix'd,
> but I don't buy the arguments in that bug. Plus, we are already moving most,
> if not all, of our app-global UI that used to be windows into tabs.

It is the right move, but Faaborg and Boriss were against it (see bug 686254), maybe UX team should be asked again.

Comment 30

[Ian Nartowicz]

About dialog is already not pinned on top in Linux.  Or am I missing something?  It isn't modal and it doesn't make any attempt to be.  It persists if the opener is closed.  Which window would you like to force a tab into in that case?

Is this yet another poorly thought out attempt to force new tabs on people even when their configuration options say otherwise?  I can see a case for doing this if the options are set to force new windows into tabs (but a new tab in which window, since the about dialog is not modal) but otherwise let it be a window.  See bug 263433 to make text-link widgets respect the "open new windows in a tab" preference.

Comment 31

[baffclan]

cannot reproduce with Firefox-nightly/Win32

Mozilla/5.0 (Windows NT 5.1; rv:21.0) Gecko/20130120 Firefox/21.0

Comment 32

[baffclan]

-> v.