Failure when we move an icon quickly over panels

VERIFIED FIXED

Status

Firefox OS
Gaia::Homescreen
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: nbp, Assigned: julienw)

Tracking

({b2g-testdriver})

unspecified
ARM
Gonk (Firefox OS)
b2g-testdriver

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 705706 [details]
Clo*c*king device.

STR:
 - Move the "Clock" icon to a newly created right-most panel.
 - Move the "Music" icon to a newly created right-most panel (after the one with the clock).
 - Move the "Clock" to any where else.

Expected:
 - Leave a blank panel, while moving icons.
 - Remove the panel after finishing the edition.

Seen:
 - … look at the screenshot …

Comment 1

5 years ago
Can't reproduce this issue.
can you provide a video? or try on latest build?
please renom if reproduced
blocking-b2g: shira? → ---
Flags: needinfo?(nicolas.b.pierron)
I was able to reproduce it on the last test-driver update.  So I renominate this bug.

Now, the problem is that I am no longer able to reproduce it to take a video, since I had to restart my phone to get rid of the extra icon and that it does not boot anymore …
blocking-b2g: --- → shira?
Flags: needinfo?(nicolas.b.pierron)
I reproduce this.

The last step is not necessary, you merely need to long press the clock icon to enter edit mode.
The error shown in logcat is :

E/GeckoConsole(  378): [JavaScript Error: "TypeError: children[i] is undefined" {file: "app://homescreen.gaiamobile.org/js/page.js" line: 605}]

IMHO it could even be TEF+ but I let drivers choose.
Taking.
Assignee: nobody → felash

Updated

5 years ago
blocking-b2g: shira? → tef?
Actually the title is not precise enough: I've seen that this can happen in other cases too. However the STR provided by Nicolas makes it easily reproducible.
Hi Julien, do you want that I take a look? thanks

the STR are perfect!
here 

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/page.js#L583

if (originIcon && targetIcon && this.olist.children.length > 1) {

and works fine
Cristian, as I said earlier, it happens also in other situation; for example, i've seen it with length == 6 :)

I believe the caclulation inside "animate" is wrong, I'm currently trying to understand this and I'll most probably have a patch later today (or maybe tomorrow if I'm brave enough to write unit tests) :) I'll ask you for review !
OK, in that case you are right. I haven't seen it in other situations. Perfect, I can review it, thanks a lot
blocking-b2g: tef? → tef+
Ok, the correct STR is:
* don't be on the last panel, but any other panel with icons is fine
* long press to go in edit mode
* drag and drop an icon to the right, quickly go over the next panel and land it to the panel after
* go back to the previous panel and long press on one icon

Expected:
* as usual, we want to be able to move this icon

Actual:
* it stays at the same place in a bigger size, even when you switch panels, and the Homescreen doesn't answer to event anymore.

This is serious because we actually go quickly over panels if we don't move the finger after moving to one panel.

This doesn't happen when we go back however.
blocking-b2g: tef+ → tef?
Summary: Removing the last icon of a middle panel cause moved icons to remain at the last touch position. → Failure when we move an icon quickly over panels
Did you unintentionally renom?
of course not, sorry about that..
Created attachment 707592 [details] [diff] [review]
patch v1

Apply the patch with |git am|.
See also the PR in https://github.com/mozilla-b2g/gaia/pull/7847.

We keep the list of icons that we animated so that we can remove the animation
exactly for the same list.

Also slightly refactored dragdrop's `overIconGrid` function. We can do more but
I didn't want to do more in a TEF+ bug.

Also fixed a bug occurring when the user tried to move an icon below the dock.
---
 apps/homescreen/js/dragdrop.js |   56 +++++++++++++++++++++-------------------
 apps/homescreen/js/page.js     |   33 +++++++++++++----------
 2 files changed, 48 insertions(+), 41 deletions(-)

Didn't add any test because I couldn't make it work with the animationend event.
Attachment #707592 - Flags: review?(crdlc)
Created attachment 707639 [details] [diff] [review]
patch v2

Fixed most remarks, thanks !

Apply the patch with |git am|
PR: https://github.com/mozilla-b2g/gaia/pull/7847

We keep the list of icons that we animated so that we can remove the animation
exactly for the same list.

Also refactored dragdrop's `overIconGrid` function. We probably can do more but
I didn't want to do more in a TEF+ bug. I tried to fail fast with boolean values
when before we were sometimes doing expensive checks before checking these bool.

Also fixed a bug occurring when the user tried to move an icon below the dock.

Last but not least, we now don't try to animate when there is only one icon
(that is the icon being dragged) in a page.
---
 apps/homescreen/js/dragdrop.js |   94 ++++++++++++++++++----------------------
 apps/homescreen/js/page.js     |   55 ++++++++++++++++-------
 2 files changed, 83 insertions(+), 66 deletions(-)
Attachment #707592 - Attachment is obsolete: true
Attachment #707592 - Flags: review?(crdlc)
Attachment #707639 - Flags: review?(crdlc)
thanks a lot, reviewing
Status: NEW → ASSIGNED
Comment on attachment 707639 [details] [diff] [review]
patch v2

All works fine, great work! thanks a lot
Attachment #707639 - Flags: review?(crdlc) → review+
blocking-b2g: tef? → tef+
https://github.com/mozilla-b2g/gaia/commit/7055b7908825a4c248b053c0526b748a10554b1d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-b2g18: --- → affected
Resolution: --- → FIXED
status-b2g18-v1.0.0: --- → fixed

Comment 19

5 years ago
This issue does not reproduce on Unagi build 20130214070203 with Dec 5th Kernel
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d1288313218e
Gaia: 6544fdb8dddc56f1aefe94482402488c89eeec49

No errors happen when moving an icon quickly over panels.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.