event.preventDefault() is called unexpectedly in apps/system/js/browser_context_menu.js

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: vingtetun, Assigned: vingtetun)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 8375507 [details] [diff] [review]
bugXXXXXX.patch

Event.preventDefault() should be called only if there is a UI that is shown. Otherwise it prevent a click to fire on the app for any long tap.

I just removed the extra preventDefault() since there is already one in the show() method.
Attachment #8375507 - Flags: review?(kgrandon)
Comment on attachment 8375507 [details] [diff] [review]
bugXXXXXX.patch

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

Is there any way we could add a test for this? Maybe just a unit test? We have:  ./apps/system/test/unit/browser_context_menu_test.js
Attachment #8375507 - Flags: review?(kgrandon) → review+
(In reply to Kevin Grandon :kgrandon from comment #1)
> Comment on attachment 8375507 [details] [diff] [review]
> bugXXXXXX.patch
> 
> Review of attachment 8375507 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is there any way we could add a test for this? Maybe just a unit test? We
> have:  ./apps/system/test/unit/browser_context_menu_test.js

I have a long tail of patches to land that will need some tests. So I will try to provide some since this small line can break a lot of stuffs.
Send something to Travis with some tests.
https://github.com/mozilla-b2g/gaia/commit/9bb31f0da81f92f0048207ea9cf51628bb1183a5

Merged with tests.
Assignee: nobody → 21
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.