1030713 - [Collection app] Web results get wrongfully deduped

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Ran Ben Aharon [:ranbena] EverythingMe]

STR:
1. Install Smart Collection "Utilities"
2. Open "Utilities" SC
3. Wait till 7 web results appear underneath the installed apps.
4. Long press "Google Keep" and click "Add to top of collection".
5. Grid items are re-rendered. "Google Keep" appears pinned to top.

Expected: 6 web results appear - only "Google Keep" is deduped.
Actual: 4 web results appear - all 3 "Google" web results have been deduped ("Google Keep", "Google Tasks" and "Google Translate")

Comment 1

[Jason Smith [:jsmith]]

Wrong behavior. We probably should block on this.

Comment 2

[Kevin Grandon :kgrandon]

Nice find. We knew we would have to adjust our logic a bit for this, so this is not surprising. Will take and write a test for this case.

Comment 3

[Kevin Grandon :kgrandon]

Attachment: Github pull request

This is not perfect, but it adds some tests so for future iteration we can make adjustments and not worry about these. There may be a larger refactoring that's possible now that we have these tests cases, but for 2.0 I would just like to do the safest thing.

Before we were matching 'Google' against all parts of the subdomain. For this type of matching we now increase the minimum number of characters so we will no longer hit this path. It is possible that this causes problems with other domains (and I could probably construct one), but I would prefer to land until we have additional datapoints as to where this might cause a problem.

Comment 4

[Ran Ben Aharon [:ranbena] EverythingMe]

Comment on attachment 8446786 [details] [review]
Github pull request

It works well on the STR I described. Not sure about the cases we didn't encounter yet :/

I'm glad you're planning on improving this in the future :)

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8446786 [details] [review]
Github pull request

I'll go with Ran's review here for now as I'm already asking Dale for other stuff. Dale feel free to leave any review/feedback here if you want. Thanks!

Comment 6

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/b8f36518696f3191a56e4f33447ee9d6ec820da1

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 8446786 [details] [review]
Github pull request

This is needed for the vertical homescreen.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Shally Li（leave from Mozilla）]

This issue has been successfully verified on Flame 2.1&2.0.
See attachment: verified_v2.1.MP4.
Reproduce rate: 0/5.

Flame 2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141201001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141201.034405
FW-Date         Mon Dec  1 03:44:15 EST 2014
Bootloader      L1TC00011880

Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141201000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141201.034308
FW-Date         Mon Dec  1 03:43:18 EST 2014
Bootloader      L1TC00011880

Comment 10

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.MP4