Closed Bug 959199 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Panning and Zooming, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox27 --- unaffected
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: julienw, Assigned: daleharvey)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
Note that the bug does not happen when I disable APZC. (I tested a consequence of this in the SMS app).
Blocks: gaia-apzc
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 ?
Flags: needinfo?(dale)
(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?
(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 :)
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
Flags: needinfo?(dale)
Attached patch 959199.patchSplinter Review
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
Attachment #8359732 - Flags: review?(bugmail.mozilla)
Attachment #8359732 - Flags: feedback?(felash)
Comment on attachment 8359732 [details] [diff] [review]
959199.patch

Fixes the issue for me :) Both in the Browser and the SMS app.
Attachment #8359732 - Flags: feedback?(felash) → feedback+
See the blocking bug for the ongoing QA analysis - still waiting for 1.3 confirmation.
blocking-b2g: --- → 1.4?
Attachment #8359732 - Flags: review?(bugmail.mozilla) → review+
This will need uplifting to aurora as well, so please request aurora approval on the patch when you land it.
Blocks: 942929
blocking-b2g: 1.4? → 1.3?
blocking-b2g: 1.3? → 1.3+
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:
Attachment #8359732 - Flags: approval-mozilla-aurora?
Comment on attachment 8359732 [details] [diff] [review]
959199.patch

This already 1.3+, so this already has approval to land.
Attachment #8359732 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e8c5f963bb59
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
No longer blocks: 959401
Duplicate of this bug: 959401
You need to log in before you can comment on or make changes to this bug.