Must click twice to open link if fixed positioned div is removed on onmouseup: clicks pass through: Open the testcase, click on the right div, then click on the left link: the link doesn't open. It actually clicks through it as if it didn't exist, and the click reaches the window directly. 1. If you change the event from mouseup to click everything is right again. 2. If you change the position fixed of the link to position absolute another bug happens: the browser starts to select text as if you had left the mouse button down. 3. If you prevent the default action of the mousedown event then the error doesn't happen (the click is received correctly and the selection doesn't start). Note that while this is effective in this testcase in our internal application preventing the default action on mousedown only solved the selection problem without solving the click-through one. This was extremely hard to reproduce as a testcase, this was hurting our application badly (we have a windowing system and clicks often reached the window behind the one you clicked on). This is not 100% reproduceable in our application although it seems to be in the testcase. This was tested in Linux and Windows in both Firefox 2.0.x and today's: "3.0a9pre" firefox build.

I noticed something extra: if you click on the right div, focus on some other application and then go back to firefox you will notice that firefox is actually selecting text, as if you had the left button pressed.

This is just a quick hack to show where the problem is. Need to think about this a bit, but would be great to have this fixed in 1.9/ff3.

This is not so ugly and should be pretty safe.

- // All the events we handle below require a frame. - if (!mCurrentTarget) { - if (NS_EVENT_NEEDS_FRAME(aEvent)) { - NS_ERROR("Null frame for an event that requires a frame"); - return NS_ERROR_NULL_POINTER; - } + // Most of the events we handle below require a frame. + // Add special cases here. + if (!mCurrentTarget && + aEvent->message != NS_MOUSE_BUTTON_UP) { return NS_OK; So we're now returning early for focus events with no frame? Is that change intended? Would it make more sense to just always drop mouse grabbing on MOUSE_BUTTON_UP in nsViewManager?

Comment on attachment 286666 [details] [diff] [review] possible patch >- // All the events we handle below require a frame. >- if (!mCurrentTarget) { >- if (NS_EVENT_NEEDS_FRAME(aEvent)) { >- NS_ERROR("Null frame for an event that requires a frame"); >- return NS_ERROR_NULL_POINTER; >- } >+ // Most of the events we handle below require a frame. >+ // Add special cases here. >+ if (!mCurrentTarget && >+ aEvent->message != NS_MOUSE_BUTTON_UP) { > return NS_OK; > } Note the |return NS_OK;| That is called if (!mCurrentTarget) and !NS_EVENT_NEEDS_FRAME(aEvent) I think I want to change PostHandle to be called anyway always, even though frame gets deleted. Hmm, but could I perhaps remove some view handling code in nsFrame+subclasses and stop capturing mouse event always when mouseup...That is riskier though, I think.

(In reply to comment #5) > even though even if

> That is riskier though, I think. Actually I don't think it's that risky, if anything it will catch other cases where we don't release mouse grabbing appropriately.

Comment on attachment 286666 [details] [diff] [review] possible patch Will post a new patch

Roc, I'd like have the |viewMan->GrabMouseEvents(nsnull, result);| in ESM, mainly because of problematic cases like Bug 373344. Handling similar situation in ViewManager::HandleEvent would mean copying patch from Bug 373344 to viewmanager.

I don't get it. You can hold a strong reference to the viewmanager (in fact, it's HandleEvent already does) so it doesn't matter if a frame or view goes away.

HandleEvent keeps strong pointer to presshell, not to viewmanager (though maybe the caller keeps strong pointer to viewmanager). But do you want GrabMouseEvents(nsnull, result); to be called, event if MouseEventGrabber != view ?

Sorry, yes. > But do you want GrabMouseEvents(nsnull, result); to be called, event if > MouseEventGrabber != view ? Yes.

I prefer the previous version. Here I add mouse_up handling to viewmanager, but some of the handling is still in ESM; shell->FrameSelection()->SetMouseDownState(PR_FALSE) must be called. And I wouldn't want to make viewmanager to depend on presshell or selection handling. Frankly, I don't understand what is so wrong calling GrabMouseEvents(nsnull) in ESM.

Comment on attachment 286838 [details] [diff] [review] proposed patch OK OK, I'm convinced.

Comment on attachment 286838 [details] [diff] [review] proposed patch This is a small change and would be great to have fixed in 1.9.

Does this patch also fix the sticky selection issue in this case when you have clicked on the green div?

..because I think testcase2 is basically what the first issue in 390833, comment 6 is talking about, which would mean this should be blockin1.9+. (also, is there a relation to bug 333175 regarding this bug?)

The patch fixes also testcase2. And yes, I think this fixes bug 333175. Not (yet) sure about bug 390833.

I backed out this because of regressions. Perhaps this should go in with some version of Bug 373344.

This does fix bug 390833, well at least the select all behavior when sub windows are closed. Since its been backed out, it doesn't :(

Now I see what the problem probably is. This needs the nsWeakView patch, after that I can upload something better...

This way if view gets deleted when dispatching click, we don't use dead aView.

(In reply to comment #24) > Created an attachment (id=289485) [details] > get viewmanager from shell This fix works for me, and doesn't cause bug 403662 like the previous patch did (tested on Mac).