Open Bug 1204440 Opened 4 years ago Updated 3 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

40 Branch
defect
Not set

Tracking

()

Tracking Status
firefox44 --- wontfix
firefox51 --- affected

People

(Reporter: nicolas, Assigned: mconley, NeedInfo)

References

Details

(Keywords: testcase)

Attachments

(1 file)

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
Component: Untriaged → Tabbed Browser
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/
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 ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase-wanted
(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
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.
(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.
I think this reproduces the issue properly.

Likely we should obey the setting when the app uses window.open() on a command+click event.
Keywords: testcase
There is no more issue on Pinterest, but still on 500px.
Andrew, do you know who could help us with that? thanks
Flags: needinfo?(overholt)
Flags: needinfo?(mconley)
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!).
Flags: needinfo?(overholt)
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.
Flags: needinfo?(mconley)
Assignee: nobody → mconley
Flags: needinfo?(mconley)
This feels kind of related to bug 1310677, fwiw.
See Also: → 1310677
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.
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?
Flags: needinfo?(mconley) → needinfo?(bugs)
You need to log in before you can comment on or make changes to this bug.