959199 - [APZC-only] Calling "preventDefault" on the contextmenu event does not prevent the click event.

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Julien Wajsberg [:julienw]]

STR:
1. loads https://everlong.org/mozilla/testcase-contextmenu/ in the browser
2. longpress in the middle of the page

Expected:
* we get only the contextmenu event

Actual:
* we get both the contextmenu and the click event

qawanted: please test the STR in 1.3, I only tested in 1.4. Thanks !

Assigning to Dale per request.

Comment 1

[Julien Wajsberg [:julienw]]

Note that the bug does not happen when I disable APZC. (I tested a consequence of this in the SMS app).

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

In the browser ? How that's interesting because the codepath for the browser should not depends on APZC.

Dale do you have any ideas about what's going on ?

Comment 3

[Julien Wajsberg [:julienw]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #2)
> In the browser ? How that's interesting because the codepath for the browser
> should not depends on APZC.

I thought the Browser used APZC since ages?

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Julien Wajsberg [:julienw] from comment #3)
> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #2)
> > In the browser ? How that's interesting because the codepath for the browser
> > should not depends on APZC.
> 
> I thought the Browser used APZC since ages?

Yes, this is what it is funny that it regressed only now :)

Comment 5

[Dale Harvey (:daleharvey)]

Ugh, The original patch was rather large and needed rebased a few times http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#1626 should be |mContextMenuHandled = Dispatch....|

we have unit tests for the gesturedetector, but nothing thats testing the actual content events with apzc enabled

Comment 6

[Dale Harvey (:daleharvey)]

https://bugzilla.mozilla.org/show_bug.cgi?id=942929 would have regressed this

Comment 7

[Dale Harvey (:daleharvey)]

Attachment: 959199.patch

Should be the fix, I dont think we have any framework currently in place to test this kats? Julien if you could verify it fixes your problem it would be useful

cheers

Comment 8

[Julien Wajsberg [:julienw]]

Comment on attachment 8359732 [details] [diff] [review]
959199.patch

Fixes the issue for me :) Both in the Browser and the SMS app.

Comment 9

[Jason Smith [:jsmith]]

See the blocking bug for the ongoing QA analysis - still waiting for 1.3 confirmation.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

This will need uplifting to aurora as well, so please request aurora approval on the patch when you land it.

Comment 11

[Dale Harvey (:daleharvey)]

https://hg.mozilla.org/integration/b2g-inbound/rev/e8c5f963bb59

Comment 12

[Dale Harvey (:daleharvey)]

Comment on attachment 8359732 [details] [diff] [review]
959199.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 942929 
User impact if declined: Quite severe break to web content
Testing completed (on m-c, etc.): verified 
Risk to taking this patch (and alternatives if risky): low risk 
String or IDL/UUID changes made by this patch:

Comment 13

[Jason Smith [:jsmith]]

Comment on attachment 8359732 [details] [diff] [review]
959199.patch

This already 1.3+, so this already has approval to land.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb7bae085b9d