Dispatch middle click manually by tracking mouse

Dispatch middle click manually by tracking mouse

Tracking targets for middle button mousedown event
and fire click event for it on mouse up if necessary.

Note: The click is not finding the common parent of
mousedown and up targets for simplicty and only sends
the click to the target if both down and up happened
on the same node.

BUG=608697
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/88eb1110baafcba070e750866a343e81b6bcc524
Cr-Commit-Position: refs/heads/master@{#395707}

[Navid Zolghadr, 2016-05-09 20:43:08]

Description was changed from

==========
Dispatch middle click manually by tracking mouse

Tracking targets for middle button mousedown event
and fire click event for it on mouse up if necessary.

Note: The click is not finding the common parent of
mousedown and up targets for simplicty and only sends
the click to the target if both down and up happened
on the same node.

BUG=608697
==========

to

==========
Dispatch middle click manually by tracking mouse

Tracking targets for middle button mousedown event
and fire click event for it on mouse up if necessary.

Note: The click is not finding the common parent of
mousedown and up targets for simplicty and only sends
the click to the target if both down and up happened
on the same node.

BUG=608697
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Navid Zolghadr, 2016-05-09 21:02:07]

nzolghadr@chromium.org changed reviewers:
+ dtapuska@chromium.org

[dtapuska, 2016-05-10 12:27:15]

I'd probably call the code something like synthetic_middleclick.js

https://codereview.chromium.org/1963823002/diff/1/chrome/browser/resources/nt...
File chrome/browser/resources/ntp4/middleclick_gen.js (right):

https://codereview.chromium.org/1963823002/diff/1/chrome/browser/resources/nt...
chrome/browser/resources/ntp4/middleclick_gen.js:12: e.target.dispatchEvent(new
MouseEvent('click', e));
The middleButtonMouseDownTarget should always likely get cleared when it is up
in order to avoid a memory leak.

[Navid Zolghadr, 2016-05-10 13:17:40]

https://codereview.chromium.org/1963823002/diff/1/chrome/browser/resources/nt...
File chrome/browser/resources/ntp4/middleclick_gen.js (right):

https://codereview.chromium.org/1963823002/diff/1/chrome/browser/resources/nt...
chrome/browser/resources/ntp4/middleclick_gen.js:12: e.target.dispatchEvent(new
MouseEvent('click', e));
On 2016/05/10 12:27:15, dtapuska wrote:
> The middleButtonMouseDownTarget should always likely get cleared when it is up
> in order to avoid a memory leak.

I don't quite understand what you are suggesting here. Do you mean I should set
that to null without any check? Say for example even when mouseup with button=0
happens?
If I do that I break an edge scenario here as well which might not be important.
If you left press outside of the icon and middle press and left release and
middle release inside in the older version of Chrome that would open the app in
the new page. If I clear that variable for any mouseup it doesn't. Although that
is an edge scenario and I don't think it is at all important. Having said that,
do you want me to still clear this variable all the time on mouseup?
Note that the memory leak you are talking about is inevitable as mouse can go
outside of the page without us getting any event at all (not even the leave
event) when for example user does Alt+Tab.

[Navid Zolghadr, 2016-05-10 14:48:25]

nzolghadr@chromium.org changed reviewers:
+ thestig@chromium.org

[Navid Zolghadr, 2016-05-10 14:48:26]

ptal

[Navid Zolghadr, 2016-05-10 17:45:32]

nzolghadr@chromium.org changed reviewers:
+ rbyers@chromium.org

[Lei Zhang, 2016-05-10 19:01:41]

thestig@chromium.org changed reviewers:
+ dbeam@chromium.org

[Lei Zhang, 2016-05-10 19:01:42]

Deferring to ntp4 OWNERS

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:33:
'synthetic_middleclick.js',
Can this entire list be sorted?

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:1: // Copyright (c) 2016
The Chromium Authors. All rights reserved.
no (c)

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:3: // found in the
LICENSE file.
add blank line after.

[Navid Zolghadr, 2016-05-10 19:28:26]

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:33:
'synthetic_middleclick.js',
On 2016/05/10 19:01:42, Lei Zhang wrote:
> Can this entire list be sorted?

Sure

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:1: // Copyright (c) 2016
The Chromium Authors. All rights reserved.
On 2016/05/10 19:01:42, Lei Zhang wrote:
> no (c)

Done.

https://codereview.chromium.org/1963823002/diff/20001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:3: // found in the
LICENSE file.
On 2016/05/10 19:01:42, Lei Zhang wrote:
> add blank line after.

Done.

[Dan Beam, 2016-05-11 05:11:13]

I don't understand why we don't want to fix the platform issue. mouse up and
down on the same element with the middle mouse button used to dispatch a click
event. now it doesn't. just seems like a web platform regression or feature.

[Navid Zolghadr, 2016-05-11 12:31:53]

On 2016/05/11 05:11:13, Dan Beam wrote:
> I don't understand why we don't want to fix the platform issue. mouse up and
> down on the same element with the middle mouse button used to dispatch a click
> event. now it doesn't. just seems like a web platform regression or feature.

I totally understand your concern. Dave and Rick may be able to explain better
here. But here is what I know.

This "sending or not sending click" for middle button was an inconsistent
behavior across different browsers and it was some amount of pain to deal with
it. Mostly they had Chrome specific code to suppress this events with some
checks. I guess as part of the effort to unify the web platforms and make the
behavior of different browsers the same there were some debates on which way to
go and finally they changed the spec to match FireFox behavior of not sending
click event for middle button to mimic the right button and only let the click
event to be for primary mouse button. Anyhow, here we are trying to have that in
Chrome. So far there were a few internal Chrome pages that were broken and I
will try to fix them. But no external websites yet. I understand the fix looks
kind of hacky here as we are sending the click event again for the middle
button. But since I'm no expert in each of the areas I believe that is the
quickest fix for this and very re-usable for other parts of the code as far as I
investigated the other issues. This synthetic click event is something that we
are going to suggest to the third party developers as well if they really want
to have the click event for middle button. But the most important point is that
we achieved the same behavior on all browsers and that synthetic click code was
needed for such a use-case anyway if they want their code to work on for example
FireFox anyway.

[Rick Byers, 2016-05-12 01:29:54]

On 2016/05/11 12:31:53, Navid Zolghadr wrote:
> On 2016/05/11 05:11:13, Dan Beam wrote:
> > I don't understand why we don't want to fix the platform issue. mouse up and
> > down on the same element with the middle mouse button used to dispatch a
click
> > event. now it doesn't. just seems like a web platform regression or feature.
> 
> I totally understand your concern. Dave and Rick may be able to explain better
> here. But here is what I know.
> 
> This "sending or not sending click" for middle button was an inconsistent
> behavior across different browsers and it was some amount of pain to deal with
> it. Mostly they had Chrome specific code to suppress this events with some
> checks. I guess as part of the effort to unify the web platforms and make the
> behavior of different browsers the same there were some debates on which way
to
> go and finally they changed the spec to match FireFox behavior of not sending
> click event for middle button to mimic the right button and only let the click
> event to be for primary mouse button. Anyhow, here we are trying to have that
in
> Chrome. So far there were a few internal Chrome pages that were broken and I
> will try to fix them. But no external websites yet. I understand the fix looks
> kind of hacky here as we are sending the click event again for the middle
> button. But since I'm no expert in each of the areas I believe that is the
> quickest fix for this and very re-usable for other parts of the code as far as
I
> investigated the other issues. This synthetic click event is something that we
> are going to suggest to the third party developers as well if they really want
> to have the click event for middle button. But the most important point is
that
> we achieved the same behavior on all browsers and that synthetic click code
was
> needed for such a use-case anyway if they want their code to work on for
example
> FireFox anyway.

Yeah, the key goal here is interoperability.  But the reason this behavior is
considered better is that sites often cancel 'click' events without considering
the button state, which then breaks the browser "open in new tab" feature.  This
is a long-standing user complaint of Chrome - see http://crbug.com/255 - #5 most
starred input bug ever.

A less hacky solution would be to fire an event named "middleclick" and update
the listeners to listen for that instead of "click".  It's a reasonable argument
to say that such an event should just be added to the web platform (and it's
still possible we'll land there I think).

[Dan Beam, 2016-05-12 02:06:24]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
both of our code has all kind of things wrong with it (shadow DOM, that events
can be simulated, etc.), but this looks mildly better

document.addEventListener('mousedown', function(e) {
  if (e.button != 1)
    return;

  var mousedownElement = e.target;

  document.addEventListener('mouseup', function f(e) {
    document.removeEventListener('mouseup', f);

    if (e.button == 1 && e.target == mousedownElement)
      e.target.dispatchEvent(new MouseEvent('click', e));
  }, true);
}, true);

[dtapuska, 2016-05-12 02:12:50]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
On 2016/05/12 02:06:24, Dan Beam wrote:
> both of our code has all kind of things wrong with it (shadow DOM, that events
> can be simulated, etc.), but this looks mildly better
> 
> document.addEventListener('mousedown', function(e) {
>   if (e.button != 1)
>     return;
> 
>   var mousedownElement = e.target;
> 
>   documC

Won't this dispatch two click events if the mouse up doesn't if the mouse leaves
the window and comes back in with the mouse button released; but a mouse down/up
occur on the same element at a later time?

[Navid Zolghadr, 2016-05-12 13:56:01]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
On 2016/05/12 02:12:50, dtapuska wrote:
> On 2016/05/12 02:06:24, Dan Beam wrote:
> > both of our code has all kind of things wrong with it (shadow DOM, that
events
> > can be simulated, etc.), but this looks mildly better
> > 
> > document.addEventListener('mousedown', function(e) {
> >   if (e.button != 1)
> >     return;
> > 
> >   var mousedownElement = e.target;
> > 
> >   documC
> 
> Won't this dispatch two click events if the mouse up doesn't if the mouse
leaves
> the window and comes back in with the mouse button released; but a mouse
down/up
> occur on the same element at a later time?

Yes, I believe Dave is right. Dan, your solutions adds multiple listeners in
similar scenarios as well. Even if users doesn't leave the icon while pressed as
Dave said, if they only middle click the first time as the current tab doesn't
change they can go back to this tab and middle click again on the same icon and
that scenario also adds multiple listeners.

[Dan Beam, 2016-05-12 18:59:05]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
On 2016/05/12 02:12:50, dtapuska wrote:
> On 2016/05/12 02:06:24, Dan Beam wrote:
> > both of our code has all kind of things wrong with it (shadow DOM, that
events
> > can be simulated, etc.), but this looks mildly better
> > 
> > document.addEventListener('mousedown', function(e) {
> >   if (e.button != 1)
> >     return;
> > 
> >   var mousedownElement = e.target;
> > 
> >   documC
> 
> Won't this dispatch two click events if the mouse up doesn't if the mouse
leaves
> the window and comes back in with the mouse button released; but a mouse
down/up
> occur on the same element at a later time?

mousing up outside of the window still dispatches a mouseup event for me on
Linux (tested before I sent this)

[Dan Beam, 2016-05-12 19:08:21]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
On 2016/05/12 18:59:05, Dan Beam wrote:
> On 2016/05/12 02:12:50, dtapuska wrote:
> > On 2016/05/12 02:06:24, Dan Beam wrote:
> > > both of our code has all kind of things wrong with it (shadow DOM, that
> events
> > > can be simulated, etc.), but this looks mildly better
> > > 
> > > document.addEventListener('mousedown', function(e) {
> > >   if (e.button != 1)
> > >     return;
> > > 
> > >   var mousedownElement = e.target;
> > > 
> > >   documC
> > 
> > Won't this dispatch two click events if the mouse up doesn't if the mouse
> leaves
> > the window and comes back in with the mouse button released; but a mouse
> down/up
> > occur on the same element at a later time?
> 
> mousing up outside of the window still dispatches a mouseup event for me on
> Linux (tested before I sent this)

obviously it's possible some other listener stops the immediate propagation and
handles this earlier.

and yes, I'm not totally sure you'll get a mouseup on all platforms (which is
why I added the "// Hope we get a mouse up..." to this similar sample here --
https://bugs.chromium.org/p/chromium/issues/detail?id=255#c123), but it's very
unlikely that a user:

a) middle mousedowns on an element
b) drags their mouse out of the window and mouseups
c) WITHOUT USING THE MOUSE ON ANYTHING ELSE, middle clicks the same element

that's the set of steps you'd need to do, afaik, to get a double-click.

other crazy things are possible with the proposed code in this CL as well:

1) mousedown on element
2) mouseup outside of window
3) mousedown outside of window
4) mouseup on element

ultimately it's fairly hard to polyfill this correctly, which is why we either
need to live with the weirdness or fix it from a lower level in the platform
(<-- my preference)

[Navid Zolghadr, 2016-05-12 20:53:59]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
On 2016/05/12 19:08:21, Dan Beam wrote:
> On 2016/05/12 18:59:05, Dan Beam wrote:
> > On 2016/05/12 02:12:50, dtapuska wrote:
> > > On 2016/05/12 02:06:24, Dan Beam wrote:
> > > > both of our code has all kind of things wrong with it (shadow DOM, that
> > events
> > > > can be simulated, etc.), but this looks mildly better
> > > > 
> > > > document.addEventListener('mousedown', function(e) {
> > > >   if (e.button != 1)
> > > >     return;
> > > > 
> > > >   var mousedownElement = e.target;
> > > > 
> > > >   documC
> > > 
> > > Won't this dispatch two click events if the mouse up doesn't if the mouse
> > leaves
> > > the window and comes back in with the mouse button released; but a mouse
> > down/up
> > > occur on the same element at a later time?
> > 
> > mousing up outside of the window still dispatches a mouseup event for me on
> > Linux (tested before I sent this)
> 
> obviously it's possible some other listener stops the immediate propagation
and
> handles this earlier.
> 
> and yes, I'm not totally sure you'll get a mouseup on all platforms (which is
> why I added the "// Hope we get a mouse up..." to this similar sample here --
> https://bugs.chromium.org/p/chromium/issues/detail?id=255#c123), but it's very
> unlikely that a user:
> 
> a) middle mousedowns on an element
> b) drags their mouse out of the window and mouseups
> c) WITHOUT USING THE MOUSE ON ANYTHING ELSE, middle clicks the same element
> 
> that's the set of steps you'd need to do, afaik, to get a double-click.
> 
> other crazy things are possible with the proposed code in this CL as well:
> 
> 1) mousedown on element
> 2) mouseup outside of window
> 3) mousedown outside of window
> 4) mouseup on element
> 
> ultimately it's fairly hard to polyfill this correctly, which is why we either
> need to live with the weirdness or fix it from a lower level in the platform
> (<-- my preference)

Dan, I totally agree that having a platform support is ideal for this case.
However, the click event doesn't seem fit anymore because of the reasons already
mentioned. We will be working maybe on a new platform event or something to
support that but until then we would like to find a remedy to fix these Chrome
internal pages.

Also I understand the proposed solution in this change has a lot of problems. It
doesn't work with different nodes or stopimmediatepropagation or stuff like
that. But this is only a fix for this new tap page and that seems enough.

Regarding your new solution, I don't see what scenario this new solution handles
that the proposed solution doesn't handle? However, I can see some scenarios
that your proposed solution doesn't behave as good as the proposed solution.
Here is one. With your solution, everytime I middle click in the document (not
necessary the icon) I will have a new mouseup handler. So there is no limit on
those and they can be added as many times as user middle click in that document.
I don't know if this seems bad or not. What do you think? Also there is this
another edge scenario which Chrome doesn't send mouse up events which caused
some issue in your solution and not in the one in this CL. Just middle press on
an app icon and while pressing, Alt+tab and release it on the other window. In
this scenario you don't get the mouse up event in Linux as I tested. Now if you
go back to Chrome and middle click on the same icon all of a sudden two tab with
the same app will open. That being said, do you want me to still use your
proposed solution instead of the one in the CL?

[Navid Zolghadr, 2016-05-13 16:01:05]

On 2016/05/12 20:53:59, Navid Zolghadr wrote:
>
https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
> File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):
> 
>
https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
> chrome/browser/resources/ntp4/synthetic_middleclick.js:17: }, true);
> On 2016/05/12 19:08:21, Dan Beam wrote:
> > On 2016/05/12 18:59:05, Dan Beam wrote:
> > > On 2016/05/12 02:12:50, dtapuska wrote:
> > > > On 2016/05/12 02:06:24, Dan Beam wrote:
> > > > > both of our code has all kind of things wrong with it (shadow DOM,
that
> > > events
> > > > > can be simulated, etc.), but this looks mildly better
> > > > > 
> > > > > document.addEventListener('mousedown', function(e) {
> > > > >   if (e.button != 1)
> > > > >     return;
> > > > > 
> > > > >   var mousedownElement = e.target;
> > > > > 
> > > > >   documC
> > > > 
> > > > Won't this dispatch two click events if the mouse up doesn't if the
mouse
> > > leaves
> > > > the window and comes back in with the mouse button released; but a mouse
> > > down/up
> > > > occur on the same element at a later time?
> > > 
> > > mousing up outside of the window still dispatches a mouseup event for me
on
> > > Linux (tested before I sent this)
> > 
> > obviously it's possible some other listener stops the immediate propagation
> and
> > handles this earlier.
> > 
> > and yes, I'm not totally sure you'll get a mouseup on all platforms (which
is
> > why I added the "// Hope we get a mouse up..." to this similar sample here
--
> > https://bugs.chromium.org/p/chromium/issues/detail?id=255#c123), but it's
very
> > unlikely that a user:
> > 
> > a) middle mousedowns on an element
> > b) drags their mouse out of the window and mouseups
> > c) WITHOUT USING THE MOUSE ON ANYTHING ELSE, middle clicks the same element
> > 
> > that's the set of steps you'd need to do, afaik, to get a double-click.
> > 
> > other crazy things are possible with the proposed code in this CL as well:
> > 
> > 1) mousedown on element
> > 2) mouseup outside of window
> > 3) mousedown outside of window
> > 4) mouseup on element
> > 
> > ultimately it's fairly hard to polyfill this correctly, which is why we
either
> > need to live with the weirdness or fix it from a lower level in the platform
> > (<-- my preference)
> 
> Dan, I totally agree that having a platform support is ideal for this case.
> However, the click event doesn't seem fit anymore because of the reasons
already
> mentioned. We will be working maybe on a new platform event or something to
> support that but until then we would like to find a remedy to fix these Chrome
> internal pages.
> 
> Also I understand the proposed solution in this change has a lot of problems.
It
> doesn't work with different nodes or stopimmediatepropagation or stuff like
> that. But this is only a fix for this new tap page and that seems enough.
> 
> Regarding your new solution, I don't see what scenario this new solution
handles
> that the proposed solution doesn't handle? However, I can see some scenarios
> that your proposed solution doesn't behave as good as the proposed solution.
> Here is one. With your solution, everytime I middle click in the document (not
> necessary the icon) I will have a new mouseup handler. So there is no limit on
> those and they can be added as many times as user middle click in that
document.
> I don't know if this seems bad or not. What do you think? Also there is this
> another edge scenario which Chrome doesn't send mouse up events which caused
> some issue in your solution and not in the one in this CL. Just middle press
on
> an app icon and while pressing, Alt+tab and release it on the other window. In
> this scenario you don't get the mouse up event in Linux as I tested. Now if
you
> go back to Chrome and middle click on the same icon all of a sudden two tab
with
> the same app will open. That being said, do you want me to still use your
> proposed solution instead of the one in the CL?

ping. We need a bit more attention on this P1 issue as there are a few similar
ones I need to address based on this one before beta.
Dan, do you have any more objection having what I said in the previous comment?
Do other reviewers have any other comment?

[Dan Beam, 2016-05-13 22:27:05]

i'm worried that if we start adding imperfect polyfills, better fixes in the
platform will be de-prioritized and we'll live with subtle bugs forever.

[Navid Zolghadr, 2016-05-13 23:06:34]

On 2016/05/13 22:27:05, Dan Beam wrote:
> i'm worried that if we start adding imperfect polyfills, better fixes in the
> platform will be de-prioritized and we'll live with subtle bugs forever.

A new event for middle click in web platform is not something we can justify
only because of a few Chrome internal pages. Disabling click for middle button
also helps us see what other websites expect and how much they rely on such an
event. If there was enough justification we can work on the spec and get other
vendors on board as well. So far the only thing we saw regarding the middle
click event was that it was annoying for the third party developers and they
didn't want it. But I can assure you we have a close eye on it.

[Dan Beam, 2016-05-17 01:08:32]

On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> On 2016/05/13 22:27:05, Dan Beam wrote:
> > i'm worried that if we start adding imperfect polyfills, better fixes in the
> > platform will be de-prioritized and we'll live with subtle bugs forever.
> 
> A new event for middle click in web platform is not something we can justify
> only because of a few Chrome internal pages.

How have you determined that you've affected only a few Chrome internal pages? 
It looks like your change has only made it to canary and dev channels at this
point (and there's been a few bugs already).

Fortunately:
1) middle click is rare
2) Firefox (and the spec) agree with the new behavior, so it's likely folks that
need cross browser support already have a hack like this in place or just didn't
rely on middle click

> Disabling click for middle button
> also helps us see what other websites expect and how much they rely on such an
> event. If there was enough justification we can work on the spec and get other
> vendors on board as well. So far the only thing we saw regarding the middle
> click event was that it was annoying for the third party developers and they
> didn't want it. But I can assure you we have a close eye on it.

[Navid Zolghadr, 2016-05-17 01:59:55]

On 2016/05/17 01:08:32, Dan Beam wrote:
> On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > i'm worried that if we start adding imperfect polyfills, better fixes in
the
> > > platform will be de-prioritized and we'll live with subtle bugs forever.
> > 
> > A new event for middle click in web platform is not something we can justify
> > only because of a few Chrome internal pages.
> 
> How have you determined that you've affected only a few Chrome internal pages?

> It looks like your change has only made it to canary and dev channels at this
> point (and there's been a few bugs already).
> 
> Fortunately:
> 1) middle click is rare
> 2) Firefox (and the spec) agree with the new behavior, so it's likely folks
that
> need cross browser support already have a hack like this in place or just
didn't
> rely on middle click
> 
> > Disabling click for middle button
> > also helps us see what other websites expect and how much they rely on such
an
> > event. If there was enough justification we can work on the spec and get
other
> > vendors on board as well. So far the only thing we saw regarding the middle
> > click event was that it was annoying for the third party developers and they
> > didn't want it. But I can assure you we have a close eye on it.

This was basically our intention to fix the internal issues so that when it gets
to beta we don't have serious internal problems and then we can see what other
third parties will complain.
I will probably revert the original change before beta branch cut and look at an
alternative event before removing middle click event again.

[Navid Zolghadr, 2016-05-18 15:01:51]

On 2016/05/17 01:59:55, Navid Zolghadr wrote:
> On 2016/05/17 01:08:32, Dan Beam wrote:
> > On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > > i'm worried that if we start adding imperfect polyfills, better fixes in
> the
> > > > platform will be de-prioritized and we'll live with subtle bugs forever.
> > > 
> > > A new event for middle click in web platform is not something we can
justify
> > > only because of a few Chrome internal pages.
> > 
> > How have you determined that you've affected only a few Chrome internal
pages?
> 
> > It looks like your change has only made it to canary and dev channels at
this
> > point (and there's been a few bugs already).
> > 
> > Fortunately:
> > 1) middle click is rare
> > 2) Firefox (and the spec) agree with the new behavior, so it's likely folks
> that
> > need cross browser support already have a hack like this in place or just
> didn't
> > rely on middle click
> > 
> > > Disabling click for middle button
> > > also helps us see what other websites expect and how much they rely on
such
> an
> > > event. If there was enough justification we can work on the spec and get
> other
> > > vendors on board as well. So far the only thing we saw regarding the
middle
> > > click event was that it was annoying for the third party developers and
they
> > > didn't want it. But I can assure you we have a close eye on it.
> 
> This was basically our intention to fix the internal issues so that when it
gets
> to beta we don't have serious internal problems and then we can see what other
> third parties will complain.
> I will probably revert the original change before beta branch cut and look at
an
> alternative event before removing middle click event again.

dbeam@ I was talking to other people and we reached to this agreement that it's
better to keep suppressing the click event for middle button in beta to see what
other websites are affected and how big of a breaking change it is if at all. At
the same time we are working on the proposal for a new event to include other
buttons of the mouse.
Having said that, we'd like to have this fix for new tap page in to make sure
there is no immediate serious problems due to the internal pages in Chrome. Is
that okay with you to land this change? Can you approve that?

[Dan Beam, 2016-05-18 23:44:11]

On 2016/05/18 15:01:51, Navid Zolghadr wrote:
> On 2016/05/17 01:59:55, Navid Zolghadr wrote:
> > On 2016/05/17 01:08:32, Dan Beam wrote:
> > > On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > > > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > > > i'm worried that if we start adding imperfect polyfills, better fixes
in
> > the
> > > > > platform will be de-prioritized and we'll live with subtle bugs
forever.
> > > > 
> > > > A new event for middle click in web platform is not something we can
> justify
> > > > only because of a few Chrome internal pages.
> > > 
> > > How have you determined that you've affected only a few Chrome internal
> pages?
> > 
> > > It looks like your change has only made it to canary and dev channels at
> this
> > > point (and there's been a few bugs already).
> > > 
> > > Fortunately:
> > > 1) middle click is rare
> > > 2) Firefox (and the spec) agree with the new behavior, so it's likely
folks
> > that
> > > need cross browser support already have a hack like this in place or just
> > didn't
> > > rely on middle click
> > > 
> > > > Disabling click for middle button
> > > > also helps us see what other websites expect and how much they rely on
> such
> > an
> > > > event. If there was enough justification we can work on the spec and get
> > other
> > > > vendors on board as well. So far the only thing we saw regarding the
> middle
> > > > click event was that it was annoying for the third party developers and
> they
> > > > didn't want it. But I can assure you we have a close eye on it.
> > 
> > This was basically our intention to fix the internal issues so that when it
> gets
> > to beta we don't have serious internal problems and then we can see what
other
> > third parties will complain.
> > I will probably revert the original change before beta branch cut and look
at
> an
> > alternative event before removing middle click event again.
> 
> dbeam@ I was talking to other people and we reached to this agreement that
it's
> better to keep suppressing the click event for middle button in beta to see
what
> other websites are affected and how big of a breaking change it is if at all.
At
> the same time we are working on the proposal for a new event to include other
> buttons of the mouse.
> Having said that, we'd like to have this fix for new tap page in to make sure
> there is no immediate serious problems due to the internal pages in Chrome. Is
> that okay with you to land this change? Can you approve that?

how does this page know whether it needs your synthetic middle-click or not? 
otherwise, we might end up with double-handling of events ;(

[Navid Zolghadr, 2016-05-19 02:05:23]

On 2016/05/18 23:44:11, Dan Beam wrote:
> On 2016/05/18 15:01:51, Navid Zolghadr wrote:
> > On 2016/05/17 01:59:55, Navid Zolghadr wrote:
> > > On 2016/05/17 01:08:32, Dan Beam wrote:
> > > > On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > > > > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > > > > i'm worried that if we start adding imperfect polyfills, better
fixes
> in
> > > the
> > > > > > platform will be de-prioritized and we'll live with subtle bugs
> forever.
> > > > > 
> > > > > A new event for middle click in web platform is not something we can
> > justify
> > > > > only because of a few Chrome internal pages.
> > > > 
> > > > How have you determined that you've affected only a few Chrome internal
> > pages?
> > > 
> > > > It looks like your change has only made it to canary and dev channels at
> > this
> > > > point (and there's been a few bugs already).
> > > > 
> > > > Fortunately:
> > > > 1) middle click is rare
> > > > 2) Firefox (and the spec) agree with the new behavior, so it's likely
> folks
> > > that
> > > > need cross browser support already have a hack like this in place or
just
> > > didn't
> > > > rely on middle click
> > > > 
> > > > > Disabling click for middle button
> > > > > also helps us see what other websites expect and how much they rely on
> > such
> > > an
> > > > > event. If there was enough justification we can work on the spec and
get
> > > other
> > > > > vendors on board as well. So far the only thing we saw regarding the
> > middle
> > > > > click event was that it was annoying for the third party developers
and
> > they
> > > > > didn't want it. But I can assure you we have a close eye on it.
> > > 
> > > This was basically our intention to fix the internal issues so that when
it
> > gets
> > > to beta we don't have serious internal problems and then we can see what
> other
> > > third parties will complain.
> > > I will probably revert the original change before beta branch cut and look
> at
> > an
> > > alternative event before removing middle click event again.
> > 
> > dbeam@ I was talking to other people and we reached to this agreement that
> it's
> > better to keep suppressing the click event for middle button in beta to see
> what
> > other websites are affected and how big of a breaking change it is if at
all.
> At
> > the same time we are working on the proposal for a new event to include
other
> > buttons of the mouse.
> > Having said that, we'd like to have this fix for new tap page in to make
sure
> > there is no immediate serious problems due to the internal pages in Chrome.
Is
> > that okay with you to land this change? Can you approve that?
> 
> how does this page know whether it needs your synthetic middle-click or not? 
> otherwise, we might end up with double-handling of events ;(

I'm not sure what page you are talking about. This synthetic event is not going
to be added to all the pages.
It is for now only for this new tab page that we know it needs to handle that.
Am I missing any point here?

[Dan Beam, 2016-05-19 18:04:43]

On 2016/05/19 02:05:23, Navid Zolghadr wrote:
> On 2016/05/18 23:44:11, Dan Beam wrote:
> > On 2016/05/18 15:01:51, Navid Zolghadr wrote:
> > > On 2016/05/17 01:59:55, Navid Zolghadr wrote:
> > > > On 2016/05/17 01:08:32, Dan Beam wrote:
> > > > > On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > > > > > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > > > > > i'm worried that if we start adding imperfect polyfills, better
> fixes
> > in
> > > > the
> > > > > > > platform will be de-prioritized and we'll live with subtle bugs
> > forever.
> > > > > > 
> > > > > > A new event for middle click in web platform is not something we can
> > > justify
> > > > > > only because of a few Chrome internal pages.
> > > > > 
> > > > > How have you determined that you've affected only a few Chrome
internal
> > > pages?
> > > > 
> > > > > It looks like your change has only made it to canary and dev channels
at
> > > this
> > > > > point (and there's been a few bugs already).
> > > > > 
> > > > > Fortunately:
> > > > > 1) middle click is rare
> > > > > 2) Firefox (and the spec) agree with the new behavior, so it's likely
> > folks
> > > > that
> > > > > need cross browser support already have a hack like this in place or
> just
> > > > didn't
> > > > > rely on middle click
> > > > > 
> > > > > > Disabling click for middle button
> > > > > > also helps us see what other websites expect and how much they rely
on
> > > such
> > > > an
> > > > > > event. If there was enough justification we can work on the spec and
> get
> > > > other
> > > > > > vendors on board as well. So far the only thing we saw regarding the
> > > middle
> > > > > > click event was that it was annoying for the third party developers
> and
> > > they
> > > > > > didn't want it. But I can assure you we have a close eye on it.
> > > > 
> > > > This was basically our intention to fix the internal issues so that when
> it
> > > gets
> > > > to beta we don't have serious internal problems and then we can see what
> > other
> > > > third parties will complain.
> > > > I will probably revert the original change before beta branch cut and
look
> > at
> > > an
> > > > alternative event before removing middle click event again.
> > > 
> > > dbeam@ I was talking to other people and we reached to this agreement that
> > it's
> > > better to keep suppressing the click event for middle button in beta to
see
> > what
> > > other websites are affected and how big of a breaking change it is if at
> all.
> > At
> > > the same time we are working on the proposal for a new event to include
> other
> > > buttons of the mouse.
> > > Having said that, we'd like to have this fix for new tap page in to make
> sure
> > > there is no immediate serious problems due to the internal pages in
Chrome.
> Is
> > > that okay with you to land this change? Can you approve that?
> > 
> > how does this page know whether it needs your synthetic middle-click or not?

> > otherwise, we might end up with double-handling of events ;(
> 
> I'm not sure what page you are talking about. This synthetic event is not
going
> to be added to all the pages.
> It is for now only for this new tab page that we know it needs to handle that.
> Am I missing any point here?

there's some behavior in C++ that allows middle 'click' events.  that code is
separate from synthetic_middleclick.js, which just looks for 'mouseup' and
'mousedown' on the same event target.

if the C++ is reverted, let's say, but synthetic_middleclick.js still applies, a
middle 'click' event would be dispatched (as per the old, pre-r390195 behavior)
but then synthetic_middleclick.js will ALSO dispatch a second click.

I would like each and every event handler to run once and as expected.  we'd
need to track all special code to simulate middle click (via mouseup/down
tracking) and remove this code if the C++ allows middle click, and re-apply them
when C++ disallows middle click.

does this makes sense?  we just need to keep the C++ and JS behaviors in sync.

[Navid Zolghadr, 2016-05-19 18:07:25]

On 2016/05/19 18:04:43, Dan Beam wrote:
> On 2016/05/19 02:05:23, Navid Zolghadr wrote:
> > On 2016/05/18 23:44:11, Dan Beam wrote:
> > > On 2016/05/18 15:01:51, Navid Zolghadr wrote:
> > > > On 2016/05/17 01:59:55, Navid Zolghadr wrote:
> > > > > On 2016/05/17 01:08:32, Dan Beam wrote:
> > > > > > On 2016/05/13 23:06:34, Navid Zolghadr wrote:
> > > > > > > On 2016/05/13 22:27:05, Dan Beam wrote:
> > > > > > > > i'm worried that if we start adding imperfect polyfills, better
> > fixes
> > > in
> > > > > the
> > > > > > > > platform will be de-prioritized and we'll live with subtle bugs
> > > forever.
> > > > > > > 
> > > > > > > A new event for middle click in web platform is not something we
can
> > > > justify
> > > > > > > only because of a few Chrome internal pages.
> > > > > > 
> > > > > > How have you determined that you've affected only a few Chrome
> internal
> > > > pages?
> > > > > 
> > > > > > It looks like your change has only made it to canary and dev
channels
> at
> > > > this
> > > > > > point (and there's been a few bugs already).
> > > > > > 
> > > > > > Fortunately:
> > > > > > 1) middle click is rare
> > > > > > 2) Firefox (and the spec) agree with the new behavior, so it's
likely
> > > folks
> > > > > that
> > > > > > need cross browser support already have a hack like this in place or
> > just
> > > > > didn't
> > > > > > rely on middle click
> > > > > > 
> > > > > > > Disabling click for middle button
> > > > > > > also helps us see what other websites expect and how much they
rely
> on
> > > > such
> > > > > an
> > > > > > > event. If there was enough justification we can work on the spec
and
> > get
> > > > > other
> > > > > > > vendors on board as well. So far the only thing we saw regarding
the
> > > > middle
> > > > > > > click event was that it was annoying for the third party
developers
> > and
> > > > they
> > > > > > > didn't want it. But I can assure you we have a close eye on it.
> > > > > 
> > > > > This was basically our intention to fix the internal issues so that
when
> > it
> > > > gets
> > > > > to beta we don't have serious internal problems and then we can see
what
> > > other
> > > > > third parties will complain.
> > > > > I will probably revert the original change before beta branch cut and
> look
> > > at
> > > > an
> > > > > alternative event before removing middle click event again.
> > > > 
> > > > dbeam@ I was talking to other people and we reached to this agreement
that
> > > it's
> > > > better to keep suppressing the click event for middle button in beta to
> see
> > > what
> > > > other websites are affected and how big of a breaking change it is if at
> > all.
> > > At
> > > > the same time we are working on the proposal for a new event to include
> > other
> > > > buttons of the mouse.
> > > > Having said that, we'd like to have this fix for new tap page in to make
> > sure
> > > > there is no immediate serious problems due to the internal pages in
> Chrome.
> > Is
> > > > that okay with you to land this change? Can you approve that?
> > > 
> > > how does this page know whether it needs your synthetic middle-click or
not?
> 
> > > otherwise, we might end up with double-handling of events ;(
> > 
> > I'm not sure what page you are talking about. This synthetic event is not
> going
> > to be added to all the pages.
> > It is for now only for this new tab page that we know it needs to handle
that.
> > Am I missing any point here?
> 
> there's some behavior in C++ that allows middle 'click' events.  that code is
> separate from synthetic_middleclick.js, which just looks for 'mouseup' and
> 'mousedown' on the same event target.
> 
> if the C++ is reverted, let's say, but synthetic_middleclick.js still applies,
a
> middle 'click' event would be dispatched (as per the old, pre-r390195
behavior)
> but then synthetic_middleclick.js will ALSO dispatch a second click.
> 
> I would like each and every event handler to run once and as expected.  we'd
> need to track all special code to simulate middle click (via mouseup/down
> tracking) and remove this code if the C++ allows middle click, and re-apply
them
> when C++ disallows middle click.
> 
> does this makes sense?  we just need to keep the C++ and JS behaviors in sync.

I see what you are saying now. Absolutely.
I'll take care of that. If I revert that change I'm going to change all these
fixes back to what they were too.

[Navid Zolghadr, 2016-05-20 16:16:34]

ping!

[Dan Beam, 2016-05-24 01:57:32]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:5: var
middleButtonMouseDownTarget = null;
wrap this in:

(function() {

... code ...

})();

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:7: if (e.button == 1) {
no curlies

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:13: if (e.target ==
middleButtonMouseDownTarget)
how does this work with shadow DOM?  should you be using e.path[0]?

[Dan Beam, 2016-05-24 01:58:00]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:41: 'trash.js',
why do you need to change this list at all?

[Navid Zolghadr, 2016-05-24 02:21:25]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:41: 'trash.js',
On 2016/05/24 01:58:00, Dan Beam wrote:
> why do you need to change this list at all?

I wasn't sure to be honest with you. The target seemed related and the fact that
other sources used in new tab page also listed here I thought I need to add that
here too. Didn't I need to? Should I just change this file to what it was?

[Dan Beam, 2016-05-24 17:20:46]

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:41: 'trash.js',
On 2016/05/24 02:21:25, Navid Zolghadr wrote:
> On 2016/05/24 01:58:00, Dan Beam wrote:
> > why do you need to change this list at all?
> 
> I wasn't sure to be honest with you. The target seemed related and the fact
that
> other sources used in new tab page also listed here I thought I need to add
that
> here too. Didn't I need to? Should I just change this file to what it was?

this is just annotating dependencies for closure compilation.  you're not adding
new types or anything, so I would revert this file

[Navid Zolghadr, 2016-05-24 17:51:14]

ptal.

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/compiled_resources.gyp (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/compiled_resources.gyp:41: 'trash.js',
On 2016/05/24 17:20:46, Dan Beam wrote:
> On 2016/05/24 02:21:25, Navid Zolghadr wrote:
> > On 2016/05/24 01:58:00, Dan Beam wrote:
> > > why do you need to change this list at all?
> > 
> > I wasn't sure to be honest with you. The target seemed related and the fact
> that
> > other sources used in new tab page also listed here I thought I need to add
> that
> > here too. Didn't I need to? Should I just change this file to what it was?
> 
> this is just annotating dependencies for closure compilation.  you're not
adding
> new types or anything, so I would revert this file

Done.

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:5: var
middleButtonMouseDownTarget = null;
On 2016/05/24 01:57:31, Dan Beam wrote:
> wrap this in:
> 
> (function() {
> 
> ... code ...
> 
> })();

Done.

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:7: if (e.button == 1) {
On 2016/05/24 01:57:32, Dan Beam wrote:
> no curlies

Done.

https://codereview.chromium.org/1963823002/diff/30001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:13: if (e.target ==
middleButtonMouseDownTarget)
On 2016/05/24 01:57:32, Dan Beam wrote:
> how does this work with shadow DOM?  should you be using e.path[0]?

I don't think there is any shadow DOM here in this page. But I changed it to
what you are suggesting anyway to make it more generic.

[Dan Beam, 2016-05-24 18:17:26]

lgtm w/nits

I wanted to make this work for shadow DOM so synthetic_middleclick.js could be
moved to a more generic location later and re-applied to other pages that need
to be fixed.

https://codereview.chromium.org/1963823002/diff/50001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/50001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:5: (function() {
\n are well worth the bytes

(function() {

var middleButtonMouseDownTarget = null;

document.addEventListener('mousedown', function(e) {
  if (e.button == 1)
    middleButtonMouseDownTarget = e.path[0];
});

document.addEventListener('mouseup', function(e) {
  if (e.button != 1)
    return;

  if (e.path[0] === middleButtonMouseDownTarget)
    middleButtonMouseDownTarget.dispatchEvent(new MouseEvent('click', e));

  middleButtonMouseDownTarget = null;
});

})();

[Navid Zolghadr, 2016-05-24 18:38:23]

https://codereview.chromium.org/1963823002/diff/50001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/50001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:5: (function() {
On 2016/05/24 18:17:25, Dan Beam wrote:
> \n are well worth the bytes
> 
> (function() {
> 
> var middleButtonMouseDownTarget = null;
> 
> document.addEventListener('mousedown', function(e) {
>   if (e.button == 1)
>     middleButtonMouseDownTarget = e.path[0];
> });
> 
> document.addEventListener('mouseup', function(e) {
>   if (e.button != 1)
>     return;
> 
>   if (e.path[0] === middleButtonMouseDownTarget)
>     middleButtonMouseDownTarget.dispatchEvent(new MouseEvent('click', e));
> 
>   middleButtonMouseDownTarget = null;
> });
> 
> })();

Sure. Done.

[Dan Beam, 2016-05-24 18:48:01]

lgtm

https://codereview.chromium.org/1963823002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/90001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:18: if (e.path[0] ==
middleButtonMouseDownTarget)
btw, it probably doesn't matter, but if e.path.length was 0 and
middleButtonMouseDownTarget was null, [][0] == null // true

[Navid Zolghadr, 2016-05-24 19:22:31]

https://codereview.chromium.org/1963823002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/ntp4/synthetic_middleclick.js (right):

https://codereview.chromium.org/1963823002/diff/90001/chrome/browser/resource...
chrome/browser/resources/ntp4/synthetic_middleclick.js:18: if (e.path[0] ==
middleButtonMouseDownTarget)
On 2016/05/24 18:48:00, Dan Beam wrote:
> btw, it probably doesn't matter, but if e.path.length was 0 and
> middleButtonMouseDownTarget was null, [][0] == null // true

I added the length check as well. I guess that would make it more clear.

[Navid Zolghadr, 2016-05-24 21:44:05]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2016-05-24 21:44:05]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1963823002/#ps110001
(title: "Add length check")

[commit-bot: I haz the power, 2016-05-24 21:44:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1963823002/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1963823002/110001

[commit-bot: I haz the power, 2016-05-24 21:56:04]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2016-05-24 21:57:22]

Description was changed from

==========
Dispatch middle click manually by tracking mouse

Tracking targets for middle button mousedown event
and fire click event for it on mouse up if necessary.

Note: The click is not finding the common parent of
mousedown and up targets for simplicty and only sends
the click to the target if both down and up happened
on the same node.

BUG=608697
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Dispatch middle click manually by tracking mouse

Tracking targets for middle button mousedown event
and fire click event for it on mouse up if necessary.

Note: The click is not finding the common parent of
mousedown and up targets for simplicty and only sends
the click to the target if both down and up happened
on the same node.

BUG=608697
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/88eb1110baafcba070e750866a343e81b6bcc524
Cr-Commit-Position: refs/heads/master@{#395707}
==========

[commit-bot: I haz the power, 2016-05-24 21:57:23]

Patchset 7 (id:??) landed as
https://crrev.com/88eb1110baafcba070e750866a343e81b6bcc524
Cr-Commit-Position: refs/heads/master@{#395707}