364380 - Don't open new windows/tabs for apple events if the url is already open

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Stuart Morgan]

STR:
- Click on a link in some other app, causing it to open in Camino
- Repeat
- See lots of windows/tabs

Rather than opening new copies each time, we should find that we have it open already and pull that window/tab to the front. That's what document-based apps do with opening files, and what Safari does (Safari also appears to reload the page, but that's potentially destructive so I really don't think we should mimic that behavior).

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Am I understanding correctly: if the URL is open anywhere (e.g., bg tab in bg window), not just if the URL is already open in the fg tab in the fg window?

And are URLs with different anchors considered different URLs for purposes of this?  E.g., http://foo.com/baz.html, http://foo.com/baz.html#bar http://foo.com/baz.html#bip

(I don't disagree with not opening another tab for each same url in principle; I'm just seeking details on the proposed impl ;) )

Comment 2

[Stuart Morgan]

(In reply to comment #1)
> Am I understanding correctly: if the URL is open anywhere (e.g., bg tab in bg
> window), not just if the URL is already open in the fg tab in the fg window?

Correct; in that case it should pull forward, rather than open something new.

> And are URLs with different anchors considered different URLs for purposes of
> this?  E.g., http://foo.com/baz.html, http://foo.com/baz.html#bar
> http://foo.com/baz.html#bip

As a first cut, I'll probably consider them different since that seems like the best way to not destroy scroll position while still opening the right thing. We can revisit it later if it's a problem.

Comment 3

[Stuart Morgan]

Attachment: fix

Since I couldn't find any uses of openNewWindowOrTabWithURL:andReferrer:alwaysInFront: that we would actually want to open duplicate windows, and since we never actually did anything useful with the last two parameters, I just renamed it showURL: and added logic to check for an existing tab/window before opening something new.

Comment 4

[froodian (Ian Leue)]

Comment on attachment 249174 [details] [diff] [review]
fix

> --- src/browser/GoMenu.mm	28 Nov 2006 05:43:02 -0000	1.23
> +++ src/browser/GoMenu.mm	20 Dec 2006 00:16:28 -0000
> @@ -447,7 +447,7 @@
>        }
>      }
>      else
> -      [(MainController *)[NSApp delegate] openNewWindowOrTabWithURL:itemURL andReferrer:nil alwaysInFront:NO];
> +      [(MainController *)[NSApp delegate] showURL:itemURL];
>    }
>  }

This changes behavior - the background information was in fact used here.

STR:
1. Open downloads window
2. History->Some history item (not already open)

pre parameter removal, this would open in a new tab or window respecting the background pref; now it always loads in the foreground.  We (plural you?) should discuss the ramifications of this.

>      else {
>        // note that we're opening a new window here
> -      controller = [controller openNewWindowWithURL:inURLString referrer:aReferrer loadInBackground:loadInBackground allowPopups:NO];
> +      controller = [controller openNewWindowWithURL:aURL referrer:nil loadInBackground:NO allowPopups:NO];

I know you didn't write it, but this comment reads very strangely.  It sounds almost as if a reviewer said "r=me.  in the else, note that we're opening a new window here."  How about just changing that to "Open a new window"? (I personally like case comments like that on the same line as the else, but I know I'm in a minority.  Do as seems appropriate)

It seems like the behavior of this patch could be very annoying for the large tab-use case, since when there is more than one tab with the URL in question it always selects the tab with the first instance of any URL.  It seems more desirable to select the tab nearest to the current one - I think we can assume that they'll more frequently want a tab in close proximity to their current work space than a tab at the far left of the tab view.

Comment 5

[Stuart Morgan]

(In reply to comment #4)
> This changes behavior - the background information was in fact used here.
> 
> STR:
> 1. Open downloads window
> 2. History->Some history item (not already open)

Hm, good call. I saw that it was in a branch where there were no BWCs, which I translated to "no windows", so I didn't think the parameter mattered. Personally I think if someone is looking at just the downloads window and opens a browser window they probably want to use it, but yeah, it should be discussed.

> It seems like the behavior of this patch could be very annoying for the large
> tab-use case, since when there is more than one tab with the URL in question it
> always selects the tab with the first instance of any URL.  It seems more
> desirable to select the tab nearest to the current one - I think we can assume
> that they'll more frequently want a tab in close proximity to their current
> work space than a tab at the far left of the tab view.

I guess I was assuming that in most cases you'd only have one of a URL open under this patch, since it would prevent the accidental duplication of windows opened from files or other apps. I can see adding logic to check the frontmost window first (and I suppose the frontmost tab itself), but in the case where for some reason someone has opened exactly the same URL in multiple tabs in the same window I really don't think it matters which one we choose.

Comment 6

[Stuart Morgan]

Attachment: v2

I misunderstood the current history behavior; it's deeply broken. If I have a browser window open with the download window in front of it, there is absolutely no way that opening history items in a background tab of the frontmost (but still background) browser window is expected behavior (I'm not holding down Command, but even then it would be iffy), which is what it does under my prefs. Similarly with opening a new window behind all other windows. It's inarguably wrong now, this patch makes it much less wrong (opening a new window is what bookmarks do). Eventually the bookmark and history code should be unified and we might want to talk about nuances of what should happen with Command-loading if there's a background window, but that's all outside the scope of this bug.

This changes that use of showURL: to opening a new window directly, since that's really what we want to do; showURL: was a roundabout way of getting there.

Comment 7

[froodian (Ian Leue)]

Comment on attachment 249935 [details] [diff] [review]
v2

r=me

Comment 8

[Samuel Sidler (old account; do not CC)]

I often have several of the same URL open in multiple tabs. I typically do this with large local documents (or documents on a LAN) that don't have anchors. (Most of these tend to be large reports or legal documents with few headings.)

The patch, as attached, would be rather annoying to me since normally when I go to open a file in Camino, I want it to open again (and again and again and again). I may be in the minority, but given that I probably use more tabs than most, I thought it'd be worth a comment from my perspective.

For easier reading, my use case is as follows:
 1. Open a local web document several times near the end of my tabs list (sometimes midway through)
 2. Scroll to different points of the document in different tabs
 3. Compare various sections of the document with other, non-html/web documents
 4. While comparing, open another instance of the local web document to check another section (and not lose my position in other tabs)

As the patch stands, a new instance won't be opened. Instead, it'll open the first instance, which definitely isn't ideal for me. The first instance is usually not the one closest to the are I'm working in.

Thus, I'm opposed to this change.

I do understand that I'm in the minority, but again, it's worth hearing my perspective.

Comment 9

[Mike Pinkerton (not reading bugmail)]

I'd be tempted to agree with ss. do we have any other feedback (from users or the community) on this being a problem we need to solve (besides "safari does it")?

Comment 10

[Stuart Morgan]

I certainly want this behavior, and I've heard some personal feedback wanting it as well.

I'm really not that concerned about that use case since:
a) the "open a local web document" part of step 1 says "developer" to me, and we aren't a developer browser; most users don't open local web documents
b) there will still be a variety of ways to explicitly multi-open files (drag-and-drop to tabs, command-enter in URL bar, etc.)
c) multi-opening a local file is completely impossible in most (all?) document-based apps, so it's not really something most users will expect to happen.

I think we should be, as usual, doing what most users expect most of the time, and since multi-opening will still be possible I don't really see this as that big a deal for the minority who want to multi-open local documents.

Comment 11

[Samuel Sidler (old account; do not CC)]

(In reply to comment #10)
> a) the "open a local web document" part of step 1 says "developer" to me, and
> we aren't a developer browser; most users don't open local web documents

Or it says they used the convenient "File -> Save As..." menu item we give them and want to open said file.

> b) there will still be a variety of ways to explicitly multi-open files
> (drag-and-drop to tabs, command-enter in URL bar, etc.)

All require the browser window to be visible to do this.

> c) multi-opening a local file is completely impossible in most (all?)
> document-based apps, so it's not really something most users will expect to
> happen.

I agree that most document-based apps do this. I also agree that the steps I listed in comment 8 tend to be for more "power" users. However, without getting into a long discussion about our target audience, we have a lot of power users -- outside the Camino contributor community -- who probably have similar use cases. fwiw, Wevah mentioned that he agreed with my comment 8 as well.

I'd also present a second case for a less-than-power-user:
 1. Load Gmail (or other ajaxy web-based email)
 2. Start reading an email
 3. Leave Camino to check a reference
 4. Use the Gmail webloc you put in your dock to open a new Camino tab/window Gmail and start writing.

A user doing that could want a new tab (because they want to reference the old email but not include it all) or want the current tab/window replaced (because it doesn't matter).

I'll also note that with web apps like Gmail, there's no easy/apparent way to open "Compose" in a new tab/window (those options aren't present in the context menu). Opening by from the bookmark bar/manager or from a webloc is the easiest way to do so.

Again, I just want to present a second side of this since it will change my work flow quite a bit and I haven't seen a very compelling argument on the improvement it will make. I understand the theory, but don't feel that the issue this solves is as major as the negative impact it will have on users who are used to, and even enjoy, the current behavior.

Comment 12

[Stuart Morgan]

(In reply to comment #11)
>  4. Use the Gmail webloc you put in your dock to open a new Camino tab/window
> Gmail and start writing.

If Gmail is a user's sole/primary mail, then I think it's at least as likely that they would like a shortcut to Gmail in their dock to behave like an app in their dock, and what an app in your dock does is bring the app forward, not open a second copy.

The compelling argument is that it makes us behave more like every other application people use.  Consistency is good.  I understand the comment 8 use case, but it applies equally to TextEdit, Word, Excel, and anything else you can open a large document in, and every single one of those applications does not open a second copy when you double-click the file again.

Comment 13

[Samuel Sidler (old account; do not CC)]

(In reply to comment #12)
> The compelling argument is that it makes us behave more like every other
> application people use.  Consistency is good.  I understand the comment 8 use
> case, but it applies equally to TextEdit, Word, Excel, and anything else you
> can open a large document in, and every single one of those applications does
> not open a second copy when you double-click the file again.

This argument is weak. The reason for a behavior like this in true "document" apps is to prevent concurrent edits of one document. That doesn't apply to browsers; you're allowed to edit a query/form as many times as you want (in the case of, say, maps.google.com) and in as many tabs/windows as you want.

It's also almost impossible to tell if the page, as it exists, is the same as the one you're loading. Basing it on the URL isn't right in the case of ajaxified sites like maps.google.com where the URL doesn't even if the contents of the site do.

In addition, this could be considered data loss. Some users consider loss of back button history as "data loss" and in this case we're not even telling them whether we've sent them to a new tab/window they can close or to an existing one they need to keep open to retain their history.

That, in and of itself, is inconsistent.

Comment 14

[Stuart Morgan]

(In reply to comment #13)
> This argument is weak. The reason for a behavior like this in true "document"
> apps is to prevent concurrent edits of one document.

No it isn't.  Every viewer app has the same behavior whether they allow editing or not.  It's that way because it's the model that the OS has decided to use for documents.

Comment 15

[Samuel Sidler (old account; do not CC)]

We currently have a preference that reads:

 Open tabs instead of windows for: [ ] Links opened by other applications

That implies that, when left unchecked, links from apple events (from other applications) will open in a new window. (And, if checked, in a new tab.)

If we do, in fact, go with this change -- which, after talking with a few of the core Firefox folk, I am now strongly opposed to -- we should either change the wording of this pref to indicate that some links from other applications won't actually open in a new window/tab (i.e., only "unique" links) or remove the pref altogether and base it on another pref, which is something I am also opposed to.

Either way, we'd be changing that pref and we need to note that change for our users.

Comment 16

[Stuart Morgan]

(In reply to comment #15)
> If we do, in fact, go with this change -- which, after talking with a few of
> the core Firefox folk, I am now strongly opposed to

Could you elaborate on why?  So far your main argument seems to be "I prefer the current behavior for my specific workflow", which I really don't see as a sufficient to counter to "We should behave like just about every other OS X application".  Users are always making requests to behave a certain way because it's useful for them for some specific reason dispite being unlike the rest of the system, and we don't do it.

AJAX sites are a fair point but:
a) URLs work for a lot of sites; if they didn't we wouldn't have bookmarks (this has been discussed in other bugs)
b) As I pointed out above, for complex "web apps"--the common case of sites where the URL never changes--there is a good argument that they should come forward rather than spawn copies because that's what applications do.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #16)
> AJAX sites are a fair point but:
> a) URLs work for a lot of sites; if they didn't we wouldn't have bookmarks
> (this has been discussed in other bugs)

"Works for a lot of sites but inconsistently for some" is a pretty bad story, user-experience wise. The reason why the expectation was born was from consistency. If we can't be consistent, then there's no way for the user to form a mental model of what's going on.

As a general rule of thumb, where we can be consistently clever in our UI, interpreting user intent with a high degree of precision, we should. If we can't be consistently clever, we should be consistently dumb.

> b) As I pointed out above, for complex "web apps"--the common case of sites
> where the URL never changes--there is a good argument that they should come
> forward rather than spawn copies because that's what applications do.

Except that when I send someone a URI over email, like a link to my new house's address, I want that person to be able to click that link and see where my house is. The way a lot of web apps work, the URI stays the same while the internal AJAXy frame is changing like mad, so all of a sudden my "click here to see my house!" shows them the last address they navigated to in the mapping application.

A link -- to a local file or a web page -- is supposed to be a unique identifier of content. On a local filesystem there's a guarantee of uniqueness; something can't exist in multiple states with the same address. That's not true on today's web, and so I'm not sure we can guarantee the same predictable result that an OS can when loading a file.

I'm going to have to add my voice to the dissenters here, but I'd like to understand the full motivation behind this bug. Having two tabs open to the same page surely isn't dangerous, and I find it dubious that someone would click on a link from an email to "edit that document I had open in the web app" and thus expect to get the same page back in the same state where they left it. Is this simply a problem with having too many tabs open, or wanting a shortcut to get back to a tab that you know is open? Perhaps it's better served by a "Sort Tabs" or "Clean Up Tabs" command or function?

Comment 18

[Stuart Morgan]

(In reply to comment #17)
> > b) As I pointed out above, for complex "web apps"--the common case of sites
> > where the URL never changes--there is a good argument that they should come
> > forward rather than spawn copies because that's what applications do.
> 
> Except that when I send someone a URI over email, like a link to my new house's
> address, I want that person to be able to click that link and see where my
> house is. The way a lot of web apps work, the URI stays the same while the
> internal AJAXy frame is changing like mad, so all of a sudden my "click here to
> see my house!" shows them the last address they navigated to in the mapping
> application.

Only if they way they previously opened the map site was also by clicking on a link to your house and then navigating away. Which yes, could happen, but it's a very edge case; how many other examples can you give of AJAX sites that are repeatedly opened to a specific sub-url externally but are navigated away from?

Again, if that case is the use case that new features must pass, then we should removing bookmarking from all products, because it won't do at all what the user expects there.  I agree that consistency is very important in UI, but the ugly truth is that almost nothing is 100% consistent when dealing with the web, so I think the question is not "will we ever be wrong" but "will we do what users expect the vast majority of the time".

> I'm going to have to add my voice to the dissenters here, but I'd like to
> understand the full motivation behind this bug. Having two tabs open to the
> same page surely isn't dangerous, and I find it dubious that someone would
> click on a link from an email to "edit that document I had open in the web app"
> and thus expect to get the same page back in the same state where they left it.

The motivation for doing it with local documents is that doing anything else with documents is wrong. Even if we don't end up doing all URIs, I have yet to see any good reason that we wouldn't do it with files.

The motivation for doing it with general URIs is twofold:
a) Be consistent with file handling
b) Prevent spawning lots of extra tabs that the user very likely doesn't want.

There are lots of examples of b) that I've encountered that make me want to change the behavior. Off the top of my head:
- accidentally double-opening links from NNW because of a stray click, or because I got interrupted and couldn't remember what I had already opened
- the increasingly popular practice of having apps open a help page by tossing a URL over to the browser; when I want help again I don't want a new copy of the page, and I don't want to change apps and dig through the rest of my pages to find it again
- knowing that I opened a link someone emailed me, but not remembering that I left it open (or again, not wanting to dig through everything I've opened); if it were a file someone sent my I would just open it again and know that it would work

That's the bug-fixy part, which is the primary reason I filed this; I want it to do what my experience with OS X leads me to expect at a gut level, it doesn't, I find that very frustrating, and I'm not the only one. The bonus is that it enables things that are currently impossible, like having a Gmail launcher with apps like QuickSilver the way other users of those tools have a Mail.app launcher. So again, I think it will improve behavior with most AJAXy web-app sites, not regress them.

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This is not blocking 1.1, because we're far from a meeting of the minds here, and we need to focus our energies on fixing bigger, more important bugs, rather than continuing to argue this.

We can revisit this (including its target) *after* 1.1 has shipped.

Comment 20

[Stuart Morgan]

There's a functional patch here; all it needs is a decision one way or the other. I don't see how it will be easier to decide later than now.

Comment 21

[Mike Pinkerton (not reading bugmail)]

My $1.25 is to go ahead with this patch. my guess is that far more people will find it helpful than confusing.

Comment 22

[Mike Pinkerton (not reading bugmail)]

+  NSArray* tabViewItems = [self tabViewItems];
+  for (unsigned int i = 0; i < [tabViewItems count]; i++) {
+    id tab = [tabViewItems objectAtIndex:i];
+    if ([[(BrowserWrapper*)[tab view] getCurrentURI] isEqualToString:aURL]) {
+      return i;
+    }
+  }

is there any way we can do a hash lookup rather than sequentially looking for the bookmark? It seems likely that for people with lots of windows with lots of tabs, this will slow things down for the (arguably) "normal" case of no window/tab having the url in it.

Comment 23

[Mike Pinkerton (not reading bugmail)]

oops. s/bookmark/url

Comment 24

[Stuart Morgan]

(In reply to comment #22)
> is there any way we can do a hash lookup rather than sequentially looking for
> the bookmark? It seems likely that for people with lots of windows with lots of
> tabs, this will slow things down for the (arguably) "normal" case of no
> window/tab having the url in it.

I opened ~50 windows, and opened a hundred or so different tabs each in a bunch of them. I selected a URL in TextEdit that wasn't open in any of them, and Sharked pressing Option-Esc Command-Option-U Option-Esc as fast as possible. I did this five or six times, and not once could I find this code in the sample.

I don't really think adding the complexity of maintaining that hash is worthwhile given that there doesn't appear to be a real performance issue.

Comment 25

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 249935 [details] [diff] [review]
v2

sr=pink

*shrug* ok.

Comment 26

[Stuart Morgan]

Checked in on trunk and MOZILLA_1_8_BRANCH with one added openNewWindowOrTab:... -> showURL: for a call that appeared after the last patch was made.

Comment 27

[Stuart Morgan]

Attachment: v3

As checked in.