1051100 - [B2G] [Homescreen] Multiple lines are displayed when moving apps to the bottom of the Homescreen

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[Seth Miko [:SMiko]]

Attachment: photo.JPG

Description:
When quickly moving apps to the bottom of the Homescreen, 2-4 lines are displayed

Repro Steps:
1) Update a Flame to 20140808000201
2) Hold down on an application to enter edit mode.
3) Quickly drag the application to the bottom corner of the Homescreen 

Actual:
2-4 lines are displayed

Expected:
1 line is displayed

Flame 2.0

Environmental Variables:
Device: Flame 2.0 (319mb)
Build ID: 20140808000201
Gaia: 8d4599d18fbfc41f88ea494ab9cce0bb99cffac3
Gecko: aad73d079368
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Keywords:
homescreen, layout, line, edit mode, 

Repro frequency:100%

See attached: (screenshot)

Comment 1

[Seth Miko [:SMiko]]

Attachment: another variation/ same issue

Comment 2

[Seth Miko [:SMiko]]

Attachment: VectorLogcat.txt

Comment 3

[Seth Miko [:SMiko]]

Comment on attachment 8470341 [details]
VectorLogcat.txt

please ignore this attachment

Comment 4

[Seth Miko [:SMiko]]

This issue DOES repro on Flame 2.1 (319mb), buri 2.1, Flame 2.0 (512mb), and buri 2.0

Flame 2.1 (319mb)

Environmental Variables:
Device: Flame 2.1 Master (319mb)
BuildID: 20140808040304
Gaia: c45627132ae7f00026e361a14d5d084a1236af24
Gecko: 18f408a5984e
Version: 34.0a1 (2.1 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

buri 2.1

Environmental Variables:
Device: Buri Master
Build ID: 20140807193101
Gaia: c45627132ae7f00026e361a14d5d084a1236af24
Gecko: 18f408a5984e
Version: 34.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0 (512mb)

Enviromental Variables:
Device: Flame 2.0 (512mb)
BuildID: 20140808000201
Gaia: 8d4599d18fbfc41f88ea494ab9cce0bb99cffac3
Gecko: aad73d079368
Version: 32.0 (2.0)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

buri 2.0

Environmental Variables:
Device: Buri 2.0
Build ID: 20140808003002
Gaia: 8d4599d18fbfc41f88ea494ab9cce0bb99cffac3
Gecko: aad73d079368
Version: 32.0 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

This issue does NOT repro on Flame 4.1, buri 4.1, or buri 3.1 as vertical sections of the Homescreen did not exist.

Comment 5

[KTucker [:KTucker][Inactive 3/4/2016]]

In comment 4 you stated 4.1 should have been 1.4. Also, you should use ? for the triage tag when you need info me.

Please attach a video to assist the development team in reproducing this issue.

Comment 6

[Seth Miko [:SMiko]]

Video: http://youtu.be/dnXHU4I6q2I

Comment 7

[KTucker [:KTucker][Inactive 3/4/2016]]

Seems minor not a blocking issue.

Comment 8

[Jason Smith [:jsmith]]

If you exit edit mode, then are the lines still present or not?

Comment 9

[Cody Roesch [:croesch]]

No, unused dividing lines for the apps don't stay on the screen once edit mode is exited. The screen looks correct when not in edit mode.

Comment 10

[yifan [:yifan][:yliao]]

Attachment: pull request

The commit checks if there are only placeholders in the last row to prevent additional dividers from created.

Comment 11

[Kevin Grandon :kgrandon]

Comment on attachment 8488496 [details] [review]
pull request

Thanks for the patch! I left one note on github. Please address it if you can and re-flag me for review.

Comment 12

[yifan [:yifan][:yliao]]

Comment on attachment 8488496 [details] [review]
pull request

Thank you for the update. :) I added a comment on the PR, please see if it's valid.

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8488496 [details] [review]
pull request

Thank you for the patch. After looking a bit more, I think we could potentially land this, but the only reason I'd be hesitant if it's not a blocker is because this may be obsoleted soon, or cause a merge conflict for app grouping. If this is not going to be an issue when we have app grouping, I'd rather not cause merge conflicts for 2.2 as that is going to slow us down a bit. This work is happening in bug 1067435.

Because this may conflict with the work in that patch, or need to get ripped out soon (that should land next week), I'd like to pass this over to Chris to review as he sees fit.

If this is something we want to uplift, it may be worth landing in master first, before the big grouping prototype lands.

Comment 14

[Chris Lord [:cwiiis]]

Comment on attachment 8488496 [details] [review]
pull request

Couple of comments on github. While this almost certainly conflicts with bug 1067435, it's a nice bit of polish and I wouldn't want to hold it up (more than I already have by being slow to review - sorry!)

Comment 15

[yifan [:yifan][:yliao]]

Thank you Kevin and Chris!

No worries, evaluating the patch takes time and your review is not slow at all :) I've updated the code. Please see if the comments are clear or not.

Comment 16

[Chris Lord [:cwiiis]]

(In reply to Yi-Fan Liao [:yifan][:yliao] from comment #15)
> Thank you Kevin and Chris!
> 
> No worries, evaluating the patch takes time and your review is not slow at
> all :) I've updated the code. Please see if the comments are clear or not.

Good comment, I added one last nit-pick but it's good to go imo :)

Comment 17

[yifan [:yifan][:yliao]]

Thank you!

Sorry that I didn't notice tests fail. In order to adhere to the original tests I revised the code. The cleanItems function remains the same if it's not in edit mode. Now the check only runs when verticalhome is in edit mode.

Comment 18

[Chris Lord [:cwiiis]]

Added a comment, still looks good, still r+ :)

Comment 19

[yifan [:yifan][:yliao]]

Thank you for the feedback! I also added some explanations.

Merged into master
https://github.com/mozilla-b2g/gaia/pull/23993
TBPL:
https://tbpl.mozilla.org/?rev=76f60ee65e0205f4ad1c735cbc58143524c35512&tree=Gaia-Try