Last Comment Bug 761572 - click event not fired if element has been hidden by mouseup
: click event not fired if element has been hidden by mouseup
Status: RESOLVED FIXED
: css2, regression
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
Depends on: 763014 780582
Blocks: 761483
  Show dependency treegraph
 
Reported: 2012-06-05 06:00 PDT by Clochix
Modified: 2012-08-06 05:21 PDT (History)
10 users (show)
mats: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified


Attachments
Test case (1.56 KB, text/html)
2012-06-05 06:00 PDT, Clochix
no flags Details
fix (5.07 KB, patch)
2012-06-05 22:59 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Clochix 2012-06-05 06:00:58 PDT
Created attachment 630142 [details]
Test case

I have an element with 2 event listeners:
* on mouseup, I hide the element with "display: none";
* on click, I trigger an action.

The action is no more triggered in latest nightly (BuildID: 20120603030523). "click" is not fired.
In Firefox 12 and Chromium, everything works as expected.

If I use "visibility: hidden" instead of "display: none", everything works fine.

I don't know if it's a bug or the expected behaviour.

Here's a test case: http://jsfiddle.net/b2Aw5/ (same as attached file).
Comment 1 Olli Pettay [:smaug] (vacation Aug 25-28) 2012-06-05 07:22:56 PDT
Regression range, please.
Comment 2 Mats Palmgren (:mats) 2012-06-05 10:35:56 PDT
Regression from bug 758179?
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-05 22:59:26 PDT
Created attachment 630457 [details] [diff] [review]
fix

The fix for bug 758179 revealed this bug. Now, setting display:none on mouseup is automatically flushed so the frame is gone by the time we get around to dispatching click/dblclick in PostHandleEvent. This bug already existed though, since Web content itself could have caused a flush after setting elements to display:none in mouseup.

This patch reverts a bit of the patch for bug 401528. I've tested the testcases in that bug and they don't regress.
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-05 22:59:46 PDT
This patch seems to fix bug 761483 as well.
Comment 5 Olli Pettay [:smaug] (vacation Aug 25-28) 2012-06-06 05:05:03 PDT
Comment on attachment 630457 [details] [diff] [review]
fix

This is quite risky, but I don't have better ideas.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-07 03:08:43 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5caad70e9632
Comment 7 Graeme McCutcheon [:graememcc] 2012-06-08 04:23:29 PDT
https://hg.mozilla.org/mozilla-central/rev/5caad70e9632

(Merged by Ed Morley)
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-06-08 04:41:42 PDT
Comment on attachment 630457 [details] [diff] [review]
fix

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

Need to get this fixed on Aurora.
Comment 9 Jason Smith [:jsmith] 2012-06-08 18:52:01 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (away June 9-19) from comment #8)
> Comment on attachment 630457 [details] [diff] [review]
> fix
> 
> Review of attachment 630457 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Need to get this fixed on Aurora.

FYI - See bug 763014. I'm seeing bustage as a result of this fix that causes reproducible crashes upon double-clicking the top portion of a FF window.
Comment 10 Alex Keybl [:akeybl] 2012-06-11 12:53:52 PDT
Comment on attachment 630457 [details] [diff] [review]
fix

[Triage Comment]
Approved for Aurora 15, since this appears to be a regression in that version.
Comment 11 Mats Palmgren (:mats) 2012-06-11 15:18:44 PDT
In that case, we also need bug 763014 on Aurora at the same time.
Comment 13 Simona B [:simonab ] -PTO- back Sept 5th 2012-08-06 04:52:32 PDT
Verified using the test case attached in the Description that the click event is fired (if element has been hidden by mouseup). 

Verified using Firefox 15 beta 3 on Windows 7, Ubuntu 12.04 and Mac OS X 10.6:
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0																			
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0

After clicking on 'Set display to none' button the background text gets selected when hovering the mouse over - filed Bug 780582.

Note You need to log in before you can comment on or make changes to this bug.