1043884 - Index letter is missing when Tapping a letter on the index of a contacts list

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Description

[Mason Chang [Inactive] [:mchang]]

If we have a list of contacts and try to quickly go to a letter, for example going from 'A' to 'Z' while scrolling the index list, or quickly tapping a single letter, we see some low resolution tiling. We also have some amount of latency between tapping an index and the contacts showing up. The first priority should be trying to improve the low resolution tiling when we scroll / tap the index list.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Video of bug

We see that we sometimes blank white, have low res tiling, and there is some latency between when we touch a letter and when we show the information. This is an issue the partner has brought up.

Comment 2

[Mason Chang [Inactive] [:mchang]]

Any thoughts on what we could do about this issue? Thanks!

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Seems to be two issues you're mentioning in this bug - chance of checkerboarding that low res tiling avoids and the latency on touching the letter, which sounds a lot like bug 1028834.

For the first issue, if it looks like we may checkerboard, because a lot of things changed and we can't reuse previous tiles, we will do the low res draw.  Otherwise, we'd probably checkerboard.  So, it's pretty much by design that this happens.

Comment 4

[Francisco Jordano [:arcturus] [:francisco]]

I agree with milan here, when changing a group we checkerboard cause we cannot reuse any tile since everything changes.

Comment 5

[Mason Chang [Inactive] [:mchang]]

Hi Jeff, will you be able to use a high speed camera and measure the time it takes between when a user lifts their finger and when the screen shows checkerboarding / low res tiling? By design, since those contact lists are not in the displayport we have to paint the new tiles. If the time between when the user lifts their finger and when the checkerboarding occurs is somewhat low, that would help UX and the graphics team determine what steps to do next. Thanks!

Comment 6

[Wayne Chang [:wchang]]

Mason,

Are you going to be tracking this bug still?

Comment 7

[Mason Chang [Inactive] [:mchang]]

Hi Wayne,

Sorry, but I don't think I will have any time to track or fix this issue in any reasonable timeframe.

Comment 8

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Evelyn, Luke,

As you can see from the video, sometime when you tap the contact index on the right side of the screen, it will only show a green circle without a character inside. Could you help to check this one? I can show you how to reproduce it

Thanks

Vance

Comment 9

[Evelyn Hung]

Vance, may I know which build/device we are aiming for?

Comment 10

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

It is on Madai 2.0, and I think Flame 2.0 has this issue as well

Thanks

Vance

Comment 11

[Evelyn Hung]

@Chens, could you help here? Thanks!

Comment 12

[Sherman Chen [:chens] (inactive)]

Confirm this issue also repro on flame master. 
I will look into how we show a green circle on right side.

Comment 13

[Sherman Chen [:chens] (inactive)]

Attachment: Contacts-EmptyCircle.png

Comment 14

[Sherman Chen [:chens] (inactive)]

Attachment: Contacts-LowReso.png

Comment 15

[Sherman Chen [:chens] (inactive)]

Vance, is attachment 8492831 [details] the same situation as you described in comment 8?

I found this could happen if you scroll to favorite or search on the top of scrollbar.

Comment 16

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Sherman Chen [:chens] from comment #15)
> Vance, is attachment 8492831 [details] the same situation as you described
> in comment 8?
> 
> I found this could happen if you scroll to favorite or search on the top of
> scrollbar.

Ya something like that, just a green circle without character inside it, but not sure if it must be search to favorite or search of the top of scrollbar. 

The way I test is to populate 200 contact data into phone via |make reference-workload-light|, then start to test by randomly tapping the index on the right, as i remember, can also reproduce in both middle or bottom part of the scrollbar

Thanks!

Comment 17

[Sherman Chen [:chens] (inactive)]

Attachment: Pull request to 2.0

Comment 18

[Sherman Chen [:chens] (inactive)]

attachment 8492975 [details] [review] could possibly solve issue mentioned in comment 8.
Note this is only wip patch and did not solve the low reso problem.

Comment 19

[Sherman Chen [:chens] (inactive)]

Vance, do we need this on 2.0?

Comment 20

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Yes this one is needed for 2.0

Thanks

Comment 21

[Sherman Chen [:chens] (inactive)]

[Blocking Requested - why for this release]: As comment 20, we need this for 2.0

Comment 22

[Etienne Segonzac (:etienne)]

Comment on attachment 8492975 [details] [review]
Pull request to 2.0

redirecting to a contact peer

Comment 23

[Dietrich Ayala (:dietrich)]

Performance improvements usually do not block the release. See the blocking criteria here: https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines

Mason, do you know if this is a regression? We auto-block on regressions.

If this is not a regression or broken new feature, we shouldn't block, and let this ride the trainz.

Comment 24

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Dietrich -

The reason to nominate this one as a blocker is because partner strongly ask for it to be fixed in order to bring better user experience to end users.

Comment 25

[Mason Chang [Inactive] [:mchang]]

To my knowledge this is not a regression. I filed this bug because the partner is interested in having this solved, but essentially it's a case where we can't paint fast enough.

Comment 26

[Rachelle Yang [:ryang][[e-mail]]]

Triage:2.0+, since this is strongly requested by Partner.

Hi Sherman, thanks for your efforts.
May I know if the WIP patch could resolve issues of low precision tiling and long latency, and also the index letter inside circle is missing ?   

Thank you very much.

Comment 27

[Sherman Chen [:chens] (inactive)]

(In reply to Rachelle Yang [:ryang][[e-mail]] from comment #26)
> Hi Sherman, thanks for your efforts.
> May I know if the WIP patch could resolve issues of low precision tiling and
> long latency, and also the index letter inside circle is missing ?   

No, as per comment 18, this patch only solve index letter missing and it's still pending review.

Comment 28

[Sherman Chen [:chens] (inactive)]

Hi Rachelle, 

Based on comment 26, I think we should update blocking-b2g flag to 2.0+ right?

Comment 29

[Rachelle Yang [:ryang][[e-mail]]]

Hi Sherman, thanks for your remind. set it to 2.0+.

BTW, do you mind sparing some time on the issues of low precision tiling and long latency of contacts?
Or do you prefer I create another bug for the issue ?
Thank you very much!

Comment 30

[Sherman Chen [:chens] (inactive)]

Yes, I think we should separate these two. Create another bug for either low precision tiling or index letter missing is fine. I can move attachment 8492975 [details] [review] to corresponding bug if necessary.

Comment 31

[Milan Sreckovic [:milan] (needinfo for best results)]

This is listed as depending on bug 1028834 - is it a blocking dependency?  In which case bug 1028834 would need to be a 2.0+ as well.

Comment 32

[Rachelle Yang [:ryang][[e-mail]]]

Hi Sherman, thanks for our suggestion.
I will keep Bug1043884 as issue of index letter missing, so you could keep your  attachment 8492975 [details] [review] here. Thanks!

BTW,Could you please kindly spare some time on bug 1028834 as well? 

Thank you very much !

Comment 33

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8492975 [details] [review]
Pull request to 2.0

Hi Sergi, I'm not sure this is right way to go, would you review this patch and give some feedback?

Comment 34

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Sherman,

Can you tag me as reviewer instead of needinfo in this case? 

Thanks!

Comment 35

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Oh, I see I am the reviewer, nevermind then :) I'll review it today.

Comment 36

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Sherman,

Could you please rebase your branch against current master?

Thanks!

Comment 37

[Sherman Chen [:chens] (inactive)]

Sure, and sorry I have to create another branch since I can't reopen the pull request, will provide new pull request later.

Comment 38

[Kevin Grandon :kgrandon]

I've gone ahead and re-opened the PR if it helps.

Comment 39

[Sherman Chen [:chens] (inactive)]

Thanks Kevin, that really helps :)

Comment 40

[Sherman Chen [:chens] (inactive)]

Hi Sergi,

I've rebase the branch to latest 2.0, would you take a look? thanks!

Comment 41

[Sherman Chen [:chens] (inactive)]

ni Sergi for comment 40.

Comment 42

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Thanks Sherman! I'll take a look.

Comment 43

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Sherman,

I flashed your patch in a peak device, and a normal 2.0 in another one. I am testing and trying to reproduce the issue but I can't see any difference. Could you let me know how to reproduce the exact issue your patch solves?

Thanks!

Comment 44

[Sherman Chen [:chens] (inactive)]

This is how I reproduce this issue: 

1. Open contacts app
2. Click on the index letter shortcut on the right side
3. Green overlay shows up with no index letter in it

I also upload a video: http://youtu.be/6THABNmyo7c

Comment 45

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Thanks Sherman, I was able to reproduce it. Your patch seems to fix it, but there is still a small issue: after touch up, the letter disappears before the circle fades out. Could we have the letter and the circle disappear at the same time?

Thanks!

Comment 46

[Sherman Chen [:chens] (inactive)]

It was intended to imitate the behavior when you scroll along the index letter shortcut. When scroll ends, the index letter in circle will disappear first, and the circle will disappear later. I can also make them disappear at the same time, so I would like to confirm which way should go for?

Comment 47

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Sherman,

I added a couple of comments to the PR; after those are fixed I think it is ready to be merged. As for the letter/circle timing, we should create another issue to address that and CC somebody from UX like Fang.

Comment 48

[Sherman Chen [:chens] (inactive)]

Hi Sergi, 

I've updated the PR and addressed those comments, could you review it again? 
And I agree we should create another bug and get UX involved to discuss the behavior.

Thanks :)

Comment 49

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8492975 [details] [review]
Pull request to 2.0

Looks good, r=me

Comment 50

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8492975 [details] [review]
Pull request to 2.0

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 
[User impact] if declined: Bad user experience, index letter not showing when tapping index shortcut.
[Testing completed]: Manual test + unit test
[Risk to taking this patch] (and alternatives if risky): Low risk and no alternatives.
[String changes made]: None

Comment 51

[Keven Kuo [:kkuo]]

Hi! Chens,

Since you are working on this case. Assign this to you.
Thanks

--
Keven

Comment 52

[bhavana bajaj [:bajaj]]

(In reply to Sherman Chen [:chens] from comment #50)
> Comment on attachment 8492975 [details] [review]
> Pull request to 2.0
> 
> NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to
> better understand the B2G approval process and landings.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): 
> [User impact] if declined: Bad user experience, index letter not showing
> when tapping index shortcut.
> [Testing completed]: Manual test + unit test
> [Risk to taking this patch] (and alternatives if risky): Low risk and no
> alternatives.
> [String changes made]: None

why is not landed on master/2.1 yet ?

Comment 53

[Sherman Chen [:chens] (inactive)]

This issue doesn't reproduce on master/2.1

Comment 54

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Merged at 280f8e33e6810e6cf43ffbae4b4b530fdbd54cd0.

Comment 55

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/a2719af81cfdb330eecf37cd6f4725ec75123aed

Comment 56

[Kai-Zhen Li [:seinlin][:kli]]

https://github.com/mozilla-b2g/gaia/commit/a2719af81cfdb330eecf37cd6f4725ec75123aed

Comment 57

[Coler]

This issue has been verified successfully on Woodduck 2.0;Flame2.0&2.2.
Reproducing rate: 0/5
See attachment: Verify_Woodduck_Cantactletter.mp4

Woodduck build version:
Gaia-Rev        d742e375aca6dc1bf3a36638000ad7f5338ef457
Gecko-Rev       d049d4ef127844121c9cf14d2e8ca91fd9045fcb
Build-ID        20141126050313
Version         32.0

Flame2.0 build version:
Gaia-Rev        99e4594c66aa3738d58b0cb44bd885a87a063b6e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f91abc6127d9
Build-ID        20141125000201
Version         32.0

Flame2.2 build version:
Gaia-Rev        824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/acde07cb4e4d
Build-ID        20141125040209
Version         36.0a1

Comment 58

[Coler]

Attachment: Verify_Woodduck_Cantactletter.MP4