1204440 - Command + click sometimes goes to the new tab even if the option is disabled

Status: NEW

(Firefox :: Tabbed Browser, defect)

Description

[Nicolas Hoizey]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504

Steps to reproduce:

- Uncheck "When I open a link in a new tab, switch to it immediately" in the settings if currently checked
- Go to 500px[1] or Pinterest[2] search page for "kittens" (should be sfw)
- Command + click on one of the photo thumbnails

[1] https://500px.com/search?q=kittens&type=photos&sort=pulse
[2] https://www.pinterest.com/search/pins/?q=kittens&term_meta[]=kittens|typed


Actual results:

The linked page opens in a new tab, which gets the focus.


Expected results:

The tab should have been opened in background, not get the focus.

The support page[3] states this:

> When you middle-click on a Web link (or hold down command while clicking with the left mouse button), the page will be opened in a new tab. That page will not be displayed and will load in a background tab. 

The link has a proper href, and I suspect there is some JavaScript event triggered, but middle-click on the same link in the same Firefox do open in a background tab.

[3] https://support.mozilla.org/en-US/kb/startup-home-page-tabs-download-settings#w_tabs

Comment 1

[Nicolas Hoizey]

500px's new profile page[1] now opens JS popins for every image instead of plain old pages, making this issue even more annoying… :-/

[1] https://iso.500px.com/introducing-the-new-500px-experience/

Comment 2

[Julien Wajsberg [:julienw] (PTO -> Dec 30)]

I reproduce the issue from comment 0 for 500px but not for pinterest.
I also see that middle clicking works as intended.

The best would be to have a simpler testcase...


(In reply to Nicolas Hoizey from comment #1)
> 500px's new profile page[1] now opens JS popins for every image instead of
> plain old pages, making this issue even more annoying… :-/
> 
> [1] https://iso.500px.com/introducing-the-new-500px-experience/

Do you have a direct link to such a profile page ?

Comment 3

[Nicolas Hoizey]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> I reproduce the issue from comment 0 for 500px but not for pinterest.
> I also see that middle clicking works as intended.
> 
> The best would be to have a simpler testcase...
> 
> 
> (In reply to Nicolas Hoizey from comment #1)
> > 500px's new profile page[1] now opens JS popins for every image instead of
> > plain old pages, making this issue even more annoying… :-/
> > 
> > [1] https://iso.500px.com/introducing-the-new-500px-experience/
> 
> Do you have a direct link to such a profile page ?

Mine, for example: https://500px.com/nhoizey

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Dec 30)]

With the devtools I see they have this code running on click:

o.prototype.onPhotoClicked = function(t) {
  var e, o, r, n, s;
  return o = this.model.getPhoto(), i() || t.metaKey || t.ctrlKey ? ("function" == typeof(n = this.model.collection).selectPhoto && n.selectPhoto(o), r = "function" == typeof(s = this.model.collection).buildBrowserUrlParams ? s.buildBrowserUrlParams() : void 0, e = o.get("url"), null != r && (e += "?" + $.param(r)), i() ? document.location = e : window.open(e, "_blank")) : App.vent.trigger(this.eventName, o, {
    navigationContext: this.model.collection
  }), !1
}

So I'd say that they call `window.open(..., "_blank")` when we ctrl + click. To be verified of course.

Comment 5

[Julien Wajsberg [:julienw] (PTO -> Dec 30)]

(In reply to Nicolas Hoizey from comment #3)
> (In reply to Julien Wajsberg [:julienw] from comment #2)
> > I reproduce the issue from comment 0 for 500px but not for pinterest.
> > I also see that middle clicking works as intended.
> > 
> > The best would be to have a simpler testcase...
> > 
> > 
> > (In reply to Nicolas Hoizey from comment #1)
> > > 500px's new profile page[1] now opens JS popins for every image instead of
> > > plain old pages, making this issue even more annoying… :-/
> > > 
> > > [1] https://iso.500px.com/introducing-the-new-500px-experience/
> > 
> > Do you have a direct link to such a profile page ?
> 
> Mine, for example: https://500px.com/nhoizey

OK, thanks, this seems to be the same behavior than on search pages.

Comment 6

[Julien Wajsberg [:julienw] (PTO -> Dec 30)]

Attachment: testcase-windowopen.html

I think this reproduces the issue properly.

Likely we should obey the setting when the app uses window.open() on a command+click event.

Comment 7

[Nicolas Hoizey]

There is no more issue on Pinterest, but still on 500px.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Andrew, do you know who could help us with that? thanks

Comment 9

[Andrew Overholt [:overholt]]

Olli and and I chatted a bit about this and he said "Pushing information that click event + ctrl is on stack when window.open is called [may be possible]. This would be mostly WindowWatcher and FF UI stuff."

Mike's going to take a look (thanks!).

Comment 10

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

This is now on my plate. A few other things are currently on fire at the moment, but I hope to get to this later in the week if possible.

Comment 11

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

This feels kind of related to bug 1310677, fwiw.

Comment 12

[Julien Wajsberg [:julienw] (PTO -> Dec 30)]

Well, not completely; in the context of this bug, it's a question that "command + click" is not obeyed. There is nothing about this in the other bug :) "command + click" makes it really clear about what the user intention is: open in the background the clicked tab.

Comment 13

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

So some JS is responding to a click event and calling window.open. Bleh.

Hey smaug, here's my plan for this one:

1) Augment AutoHandlingUserInputStatePusher so that it looks at key modifiers, and tells the ESM about it via static void StartHandlingUserInput
2) Add a static EventStateManager::CurrentUserInputModifiers that returns modifier keys if there's currently a user event being handled on the stack
3) Have nsGlobalWindow::OpenJS (or maybe nsGlobalWindow::OpenInternal?) query EventStateManager::CurrentUserInputModifiers, and pass any modifiers along to nsWindowWatcher
4) Have nsWindowWatcher pass those modifiers along to ProvideWindow. If ProvideWindow determines that we're opening a new tab, pass those modifiers along to nsIBrowserDOMWindow's openURI
5) Have the browser front end determine how to deal with any modifiers that are being passed in. From there, the front-end JS can trivially decide to open a new tab in the background if the modifiers seem appropriate.

Does that sound like a reasonable way forward? Or is there a better path that I'm not seeing?

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Sounds reasonable (tiny bit delay with replying ;) )

Comment 15

[Nicolas Hoizey]

I now have this issue in Flickr's search results:
https://www.flickr.com/search/?text=firefox

Comment 16

[Nicolas Hoizey]

It looks like the issue is not there anymore, at least on Flickr.

On the 500px page I listed when opening the bug, the behavior is now different, the linked page opens in current tab even if I click Command. I wish they knew how to develop for the Web.

On Pinterest, the issue is still there, the browser switch to the newly opened tab immediately.

So I guess Flickr did fix something on their side.

So the issue in Firefox is still there IMHO.