748740 - New tab is not opening after "Ctrl/Cmd+Click" on a link if there is "event.stopPropagation()" in the "click" handler

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Grunin Yuriy]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120312181643

Steps to reproduce:

1) Make test page with one link in it (<a href="http://google.com">Test link</a>)
2) Attach click handler to this link that stops propagation of the event:
  elem.addEventListener('click', function(e) {
    e.stopPropagation();
  }, false);
2) "Ctrl/Cmd+Click" on this link.


Actual results:

Nothing happens.


Expected results:

A new tab should be opened with "http://google.com" in it, because there is no "e.preventDefault()" in event handler.
Middle-button click works just fine.

Comment 1

[mjh563]

Confirmed on Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0

Middle click works OK
Context menu "open link in new tab" works OK
Ctrl-click does nothing.

Comment 2

[mjh563]

Attachment: Reporter's testcase

Comment 3

[pavel.shkleinik]

Please, fix this stuff

Comment 4

[pavel.shkleinik]

How can I change the status? It is definitely CONFIRMED :)

Comment 5

[pavel.shkleinik]

Is there some known workarounds for the bug?

Comment 6

[Dão Gottwald [:dao]]

Not a recent regression, no reason to track this.

Comment 7

[Dão Gottwald [:dao]]

It seems that front-end code can't do much about this. It could use a capturing click event listener, but then it wouldn't honor event.defaultPrevented.

Comment 8

[pavel.shkleinik]

Sounds sad :( Navigation in my application is fully customized and based on handling this event. Work everywhere! ))) Even in IE and Opera ;)

Comment 9

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

This is all about front-end code. It can use system event group.

Comment 10

[pavel.shkleinik]

Googling on "system event group" gave me no meaningful results. Could you provide me with some code snippet or other tip?

Comment 11

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

Firefox UI code can use system event group, not a web page.
As an example 
http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChild.js?mark=120-127#119

Comment 12

[pavel.shkleinik]

Ok, I can catch the event after the "stopPropagation", but how it can help me to keep the default "Ctrl+Click" functionality?

Comment 13

[Dão Gottwald [:dao]]

Attachment: patch

Comment 14

[Dão Gottwald [:dao]]

I get this failure with that patch:
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/places/tests/browser/browser_markPageAsFollowedLink.js | User activated visits should get a FRAMED_LINK transition. - Got null, expected 8

Comment 15

[pavel.shkleinik]

Any expectations when the issue will be resolved? =)

Comment 16

[Dão Gottwald [:dao]]

Attachment: patch

I don't know what to do about the browser_markPageAsFollowedLink.js failure. It seems that places history requires markPageAsFollowedLink to be called at a certain (wrong, by this bug's measure) point in time during the click event propagation.

Comment 17

[pavel.shkleinik]

Your answer didn't lit light on the issue at all for me ))

Comment 18

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Places just collects calls to markPageAsFollowedLink and applies the last followed flag to the next visit.
so markPageAsFollowedLink -> stored into an hash -> AddVisit uses the info
There is no specific requirement about when it happens, could be the test is too picky for when it checks?

Comment 19

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Did you verify, by chance, if we still reach the markPageAsFollowedLink call with the change? I wonder if some event change causes us to bail out earlier.

Comment 20

[Dão Gottwald [:dao]]

(In reply to Marco Bonardo [:mak] from comment #19)
> Did you verify, by chance, if we still reach the markPageAsFollowedLink call
> with the change? I wonder if some event change causes us to bail out earlier.

No I didn't! Apparently event.defaultPrevented is true, causing contentAreaClick to return early. Thanks for the hint.

Comment 21

[Dão Gottwald [:dao]]

Attachment: patch v2

(In reply to Dão Gottwald [:dao] from comment #20)
> Apparently event.defaultPrevented is true, causing contentAreaClick to return early.

Making the event listener a capturing one makes browser_markPageAsFollowedLink.js pass. I assume core code was consuming the event before us. Does this make sense?

Comment 22

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

Comment on attachment 726574 [details] [diff] [review]
patch v2

Hmm, using system group doesn't affect to preventDefault() handling, but
stopPropagation() is per group, so yes, this makes sense to me.

Comment 23

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 726574 [details] [diff] [review]
patch v2

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

sort of scary, since involves all clicks in browser, but easy to backout in case

Comment 24

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb01f41e077b

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/eb01f41e077b

Comment 26

[pavel.shkleinik]

Resolved? Seriously? You are not kidding me? =) Thanks a lot guys! I'm awaiting next FF release as never!