Gecko http://hg.mozilla.org/releases/mozilla-b2g18/rev/282b5c37cf8d Gaia e2ef782119b7e79fc62c48d36f0c36909d982988 BuildID 20130712070210 Version 18.0 leo 1. launch dialer 2. select contacts option 3. select call log option 4. select contact 5. switch back to the call log (don't hit back) 6. tap on the middle contact log person Expected: contact will open Actual: contact will open and the favorite button will activate Note: There's other variations of this bug where the wrong contact will open.

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #0) > 6. tap on the middle contact log person Can you elaborate on what you mean by this? Is this the "contacts option" button from step 2? Or a different person from the middle of the call log list? So far I have not been able to reproduce on a fresh b2g18+v1-train using reference-workload-heavy.

I can reproduce this bug by below steps. Precondition: 2~3 call logs, 1 contact at least 1. launch dialer 2. select contacts option 3. click one contact 4. select call log option 5. click the log in the middle of the screen (same position as "Add as Favorite" button of contact detail Expected: contact is openen Actual: contact is opened and favorite button is activated This is because the click event is propagate to old contact iframe improperly.

This patch should be capable to solve this problem.

(In reply to George Duan [:gduan] from comment #3) > 5. click the log in the middle of the screen (same position as "Add as > Favorite" button of contact detail Ah, that makes sense. With the reference-workload data on my Buri the "Add as Favorite" button was scrolled off the screen, so it was not in the same position. Based on your pull request it looks like the issue is in Dialer. Moving other to that category.

Comment on attachment 775307 [details] https://github.com/mozilla-b2g/gaia/pull/10961 I think Etienne is the owner for Dialer, so he's probably a good choice for reviewer.

Comment on attachment 775307 [details] https://github.com/mozilla-b2g/gaia/pull/10961 Wow, nice bug :) Thanks George for debugging this ! I'm going to send an alternative CSS patch because I want to fix the root cause, sorry for highjacking the bug :)

Here's a simple css fix, what do you think?

Comment on attachment 775581 [details] Pointer to gaia PR Hi Etienne, I tried your solution and found another bug. pre-condition: dialer is close, two call log form known contacts at least, two contacts at least 1. click dialer 2. click calllog tab first( not contact ) 3. select one log in the middle (then go to this known contact) 4. click the left-top button (then go to calllog) 5. click one log in the middle again the favorite button is activated unexpected.

I can reproduce the scenario from Comment 9. I'm a bit puzzled by this... But it looks like it's not reproducible if I remove the mouse_event_shim.js (used for the keypad and the call screen but not here so it's not preventing us from going through the STR). Any pointers? (The event actually making the contact a favorite is a trusted event, so it might be more of a timing issue.)

I think this is a good opportunity to remove mouse_event_shim.js from this app. It will improve load time and responsiveness. I'll take a look at this tomorrow.

Anthony: see also bug 861735. I've seen an email from the developer working on that bug indicating that she has already removed the shim from the communications apps. Mahsa is that correct? Etienne: I haven't looked carefully at the bug details or the patches, so I don't really know what is going on. Replacing the shim seems like the right thing to do, though, so I'm not going to try to figure this one out. The shim registers a capturing event handler on the window to discard trusted mouse events. Do the dialer or contacts apps use iframes? Maybe event dispatch is different in that case and the capturing listener doesn't get a chance to run. In any case, removing the shim, here or in bug 861735 seems like the right way to go.

I trace the log and found things as below.. 1. all events.isTrusted are false 2. click one of log will generate two click events (first from click itself, second from touchend) 3. the handleTouchEnd generate click event to #contacts-view (that's why it toggle favorite button)

Yes, it is correct. I'm still working on it though. So I don't have a SIM card to make calls, may I ask if there is any way or trick to import some call log? I would like to produce this bug and see how it works without shim. Because in the code, I see contacts uses iframe.

(In reply to Mahsa Mojtahedi from comment #14) > Yes, it is correct. I'm still working on it though. So I don't have a SIM > card to make calls, may I ask if there is any way or trick to import some > call log? I would like to produce this bug and see how it works without > shim. Because in the code, I see contacts uses iframe. I believe the reference workloads include call log data. From a gaia master checkout run: make reference-workload-heavy This will work even if your target device is 1.1, although data may be slow to render at first while databases are updated. More info here: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Platform/Gaia/Hacking_Tips_And_FAQ#Reference_Workloads

I am sorry but Etienne's patch does not work for me either... I get the same behavior as George mentions in comment 9. When going back to the Call Log using the back key and getting the details of the second entry in my Call Log (the one in the same position as the Favorite button in fact), the Favorite "tag" gets activated or deactivated :-( BTW, the original bug is solved by Etienne's patch ;-) How would you like to proceed, Etienne? Should I r+ this bug and open a new one for the back button case. Would you like to extend your patch to cover that case too? Thanks!

Since Etienne already submitted a patch ;-)

I would suggest to simply remove deps of mouse_event_shim.js for dialer app. This can prevent this bug and comment 9 .

Perfectly fine with the solution if Etienne is also still fine with it as mentioned in comment 10 ;-) Are you Etienne? :-)

Should we create a unique PR combining attachment 775581 [details] and attachment 777697 [details] ?

Ok, as I know , Etienne Segonzac (:etienne) (PTO 07/18 -> 08/05) , I will combine them into one patch.

Great! Thanks!

Comment on attachment 777697 [details] PR to master Hi George, I just noticed that in the Dialer the long press deletion (to delete the whole number typed) does not work. Either the long press of the '0' key to get a '+'. On the other hand, during a call I no longer hear the long pressing tones when activating the keypad and pressing keys. All these features work in the latest master so I guess we are kind of breaking something here ;-) Would you have a look at it please?

Clearing the needinfo for Etienne as he is on PTO and he is ok with removing mouse_event_shim.js. In attachment 777697 [details], you're only removing mouse_event_shim.js. But you also need to convert all mouse* events to touch* events. Things like mouseup, mousedown, mousemove, mouseenter, mouseleave, etc.

Since Etienne is on PTO and George is taking care of this bug ;-)

Comment on attachment 777697 [details] PR to master Replace all mouse event with touch event.

I found another bug when switching pages, and it happens no matter add this patch or not. STR: pre condition: at least one contact , one calllog for that contact 1. click contact bottom tab 2. click contact A 3. click calllog bottom tab 4. click call log from/to A 5. click top-left bottom 6. click call log from/to A 7. repeat 5, 6 expected: we can see contact A's information actual: only have contact list, and click event doesn't work anymore on contact page I think I should file a new bug for it.

Comment on attachment 777697 [details] PR to master Looking good to me ;-) Thanks George!

+1 to filling a new bug for comment 27 ;-)

Comment on attachment 777697 [details] PR to master I don't think we should use touchleave. Even though it looks supported in Gecko, it has been removed from the spec. Also, you need to update other parts of the code. For example, in swiper.js, you can't use evt.pageX directly because touch events don't have this property. It's on a touch object. With your current patch, you can't answer a call if the phone was locked, you can't swipe up. I also think all changes in oncall.js should be click events. But I'm not really sure.

Comment on attachment 777697 [details] PR to master This patch has been modified for below items. 1. support both of touch and mouse event (will depends on window.ontouchstart) 2. listen click events in oncall.js 3. remove redundant variable 4. remove mouseleave event listener

@BenKelly thank you for help. I'm on the latest version of master. I imported call log data with both heavy and light workloads. I was trying all the scenarios above in comments 1,3 and 9 and I'm not able to reproduce this bug w/o shim.

Mahsa: If you have a working patch, could you paste it here for comparison? That would help me review George's patch. George: Thanks for the summary of the changes in your latest patch, it's really helpful. I'm not sure why we want to support mouse events with your latest patch. Is there any good reason? I don't think any other app does this and it makes the code harder to read. So unless you have a good reason to do that, I'd rather only support touch events. Also, have you tested DTMF tones after removing the mouseleave event listener? I'm worried that it might regress bug 829406.

Comment on attachment 777697 [details] PR to master Patch updated for below items. 1. support both of touch and mouse event (will depends on window.ontouchstart) 2. listen click events in oncall.js 3. remove redundant variable 4. remove mouseleave/touchleave event 5. detect key change by position when touchmove/mousemove (#829406) 6. add method un utils to get x/y position of mousemove/touchmove event 7. replace if else with switch in keypad.js 8. remove comment 8 css, since it cause pb of comment 27 . In reply to comment 34, Hi Anthony, you're right, I miss bug 829406, so I update this patch for it. And the test works fine, Please let me know if any other bug you find. As I know, desktop build doesn't support touch event, so I still keep them in code as most of our other apps do. In reply to comment 33, I also tried only remove mouse_event_shim.js in latest gaia, however, we may still have problems like bug 829406 and comment 23 .

Hi George! Currently reviewing your new patch ;-) Should we set attachment 777697 [details] as obsolete?

And probably to close the associated PR to avoid "quick fingers" to click on the yummy green merge button :-p

Ups, sorry!!! I meant to set attachment 775581 [details] to obsolete and closing Etienne's PR. In fact, I am going to do it right now ;-)

Comment on attachment 777697 [details] PR to master Looking and working good to me! Thanks George!

(In reply to George Duan [:gduan] from comment #35) > As I know, desktop build doesn't support touch event, so I still keep them > in code as most of our other apps do. If B2G desktop doesn't work with touch events, we should fix that. Kevin Grandon sent a mail to dev.gaia a week or two ago. Please remove the code that handles mouse events and abstracts pageXY.

Comment on attachment 777697 [details] PR to master r- to remove the mouse abstractions. Other than that, the patch looks ok.

Comment on attachment 777697 [details] PR to master Patch updated for comment 40.

Comment on attachment 777697 [details] PR to master Great! Thanks a lot.

Merge to master. https://github.com/mozilla-b2g/gaia/commit/a883087e04418ff8a7d44f05a3e074388ce5b57b

Is it me or using the latest master (as for now) the Dialer key pressing tones are no longer played?

Please lend this bug to v1-train

In reply to comment 45 , I tried with latest gaia from master and tested with other devices... key pressing tones work fine both offline and online

Merge to v1-train https://github.com/mozilla-b2g/gaia/commit/a64f37c3327e4cf2105a16eedaf68727bb2adbde

v1.1.0hd: a64f37c3327e4cf2105a16eedaf68727bb2adbde

I just added milestone & whiteboard comment for sync-up with vendor build.