329810 - middle click on a link does not work in Camino trunk builds

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect)

Description

[Hiro]

There is no problem in Firefox.

In trunk NB 2006030800, middle click does not work on a link(<a></a>).

Reproducible: Always

Mac OS X 10.3.9
2006030800 (v1.2+)

Comment 1

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

Any idea when this broke?

Comment 2

[Chris Lawson (gone)]

Also reported here:

http://forums.mozillazine.org/viewtopic.php?t=390905

Last couple of days, maybe?

cl

Comment 3

[philippe (part-time)]

This works for me with the 2006030600 (1.2+) build
but not anymore with the 2006030800 (1.2+) build.

Comment 4

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

Confirming based on phillipe's comment, and based on that, the window is http://tinyurl.com/lbn98

That's pretty big and there are a lot of bugs there; someone with a middle-mouse button testing the 07 March trunk build would help narrow that down.

Comment 5

[philippe (part-time)]

(In reply to comment #4)

> That's pretty big and there are a lot of bugs there; someone with a
> middle-mouse button testing the 07 March trunk build would help narrow that
> down.
> 

I had problems downloading that one, mirror was hopelessly slow.
2006030700 (1.2+) works for me.
(all this on 10.4.5)

Comment 6

[carboncopy]

2006030708 - own build
on 10.4.5+ works

Comment 7

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

New window: http://tinyurl.com/hz3v3

Of those bugs, bug 234455 seems the most likely to have touched this sort of thing.

Comment 8

[Håkan Waara]

It's interesting that it works in firefox.  It makes me somehow vaguely suspect our embedding code that hooks up to this event.

Comment 9

[Håkan Waara]

It's probably that event patch...  I've commented on that bug.

Comment 10

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

This is probably a regression from bug 234455 but I don't have a mac, so I can't
really test.
Could someone point me the place where Camino's middle click handling
is implemented (that is apparently somehow different comparing to FF's 
middle click handling).

Comment 11

[Håkan Waara]

(In reply to comment #10)
> This is probably a regression from bug 234455 but I don't have a mac, so I
> can't
> really test.
> Could someone point me the place where Camino's middle click handling
> is implemented (that is apparently somehow different comparing to FF's 
> middle click handling).

See http://lxr.mozilla.org/mozilla/source/camino/src/embedding/ 

CHClickListener is what you want to look at.

Comment 12

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

Didn't find anything yet, but this sounds like some event listener is registered
either to wrong phase (capture/bubble) or wrong group (default/system).
It is also true that the order in which event handlers/default handling is
processed may have changed in some cases. I'll look at this later
today (unless someone who knows that Mac code finds the problem first ;) )

Comment 13

[Håkan Waara]

Sorry, the code you want to look at is http://lxr.mozilla.org/mozilla/source/camino/src/browser/ContentClickListener.mm

We register one such listener here: http://lxr.mozilla.org/mozilla/source/camino/src/browser/BrowserWrapper.mm#343

If I set a breakpoint in ContentClickListener::MouseDown(), then it's getting called for normal mousedowns but simply not for middle-clicks.

Comment 14

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

Hmm, I did change CheckForAndDispatchClick.
The handling of NS_EVENT_FLAG_NO_CONTENT_DISPATCH has changed...
http://lxr.mozilla.org/seamonkey/source/content/events/src/nsEventStateManager.cpp#2986

I wonder if setting nglayout.events.dispatchLeftClickOnly to false makes any 
difference. But I'm still just guessing, I'll try to find out the real
reason for this bug later today.

Comment 15

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

Hmm, now I see. There is probably aVisitor.mForceContentDispatch = PR_TRUE; 
missing either in nsGlobalWindow::PreHandleEvent
or nsWindowRoot::PreHandleChromeEvent, or actually from both places.
If someone could test whether that helps here, that'd be great.
So add aVisitor.mForceContentDispatch = PR_TRUE;
after aVisitor.mCanHandle = PR_TRUE; in those methods.

(And remove nglayout.events.dispatchLeftClickOnly false, if you already set that ;))

Comment 16

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

This is probably related to bug 329119

Comment 17

[Håkan Waara]

(In reply to comment #15)
> Hmm, now I see. There is probably aVisitor.mForceContentDispatch = PR_TRUE; 
> missing either in nsGlobalWindow::PreHandleEvent
> or nsWindowRoot::PreHandleChromeEvent, or actually from both places.
> If someone could test whether that helps here, that'd be great.
> So add aVisitor.mForceContentDispatch = PR_TRUE;
> after aVisitor.mCanHandle = PR_TRUE; in those methods.

Yes, this change seems to fix it!

Comment 18

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

Attachment: proposed patch

NS_EVENT_FLAG_NO_CONTENT_DISPATCH wasn't used in nsWindowRoot nor in 
nsGlobalWindow before 234455, so this is just putting back that functionality.
And according to Håkan this fixes the bug :)