[Metrics] Search through suggestions are not counted

RESOLVED FIXED in 2.2 S8 (20mar)

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: shinglyu, Assigned: thills)

Tracking

({verifyme})

unspecified
2.2 S8 (20mar)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
*** Description
Search by clicking the search suggestions are not counted in the app usage ping.

*** Steps to Reproduce
Pre-requisite: the search suggestion is enabled.

1. Click the rocket bar
2. Type "Mozilla"
3. Click the first search suggestions 
4. Wait for the App Usage ping to reach the metrics server

*** Expected Results
Default search provider has a search count of 1

*** Actual Results
Search count = 0

*** Other Notes
Only the searchs performed by clicking the "magnifier glass" button on the keyboard are counted. Users are equally probable to use both methods, so the search count will be underestimated

*** Reproduction Frequency


*** Build
Flags: needinfo?(thills)
(Reporter)

Comment 1

4 years ago
*** Reproduction Frequency
3/3

*** Build
  Gaia-Rev        3f032238a52f08e4c2f68a47ad065a96eb22d470
  Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9ae4bcc9b5f2
  Build-ID        20150309162503
  Version         37.0
  Device-Name     flame
  FW-Release      4.4.2
  FW-Incremental  eng.cltbld.20150309.201258
  FW-Date         Mon Mar  9 20:13:09 EDT 2015
  Bootloader      L1TC000118D0
Looks like its a broken functionality from a new metrics related feature, so blocking on this
blocking-b2g: 2.2? → 2.2+

Comment 4

4 years ago
Hi Tamara, so you are working on this? thanks.
Flags: needinfo?(thills)
(Assignee)

Comment 5

4 years ago
Comment on attachment 8576401 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master

Hi Marshall, sorry for the confusion.  I originally inadvertently posted this patch on the wrong bug.  I was wondering why the autolander didn't show up :)
Attachment #8576401 - Flags: review?(marshall)
(Assignee)

Comment 6

4 years ago
Hi Howie,

Yes, sorry I forgot to set to assigned.  I have posted a PR as well.

Thanks,
-tamara
Status: NEW → ASSIGNED
Comment on attachment 8576401 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master

Looks great! We should probably have someone that works on search take a look too..
Attachment #8576401 - Flags: review?(marshall) → review+
(Assignee)

Comment 8

4 years ago
Comment on attachment 8576401 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master

Hi Dale,

Would you be able to have a look?

Thanks,

-tamara
Attachment #8576401 - Flags: feedback?(dale)
Comment on attachment 8576401 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master

This looks great, cheers
Attachment #8576401 - Flags: feedback?(dale) → feedback+
(Assignee)

Updated

4 years ago
Assignee: nobody → thills
(Assignee)

Comment 11

4 years ago
Comment on attachment 8576401 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Search count metrics feature
[User impact] if declined: Suggested searches will not be counted and we will significantly undercount the metrics if we don't have this
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): low-medium.  Should be easy to backout as it's small.
[String changes made]:no
Attachment #8576401 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8576401 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
(Assignee)

Comment 14

4 years ago
Hi Dale,

I'm having some trouble with my suggestions_test.js when it gets uplifted to 2.2 and realized that this line (SearchProvider.ready()) is removed in master, but still exists in 2.2

master:
https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15#diff-a72719dbedcb33e670a082ee4341d876L36

2.2:
https://github.com/mozilla-b2g/gaia/blob/v2.2/apps/search/js/providers/suggestions.js#L36

It looks like bug 1131966 removes the line, but that change doesn't seem to be in 2.2.  Just wanted to know if this will be uplifted before I redo the unit test to work differently with 2.2.

Thanks!

-tamara
Flags: needinfo?(thills) → needinfo?(dale)
(Reporter)

Updated

4 years ago
Keywords: verifyme
Unfortunately, this is still hitting test failures on v2.2 even with bug 1136664 uplifted first :(. Backed out again.
v2.2: https://github.com/mozilla-b2g/gaia/commit/f52a2ac8f42489587e95a9254045683e0454b2e6

https://treeherder.mozilla.org/logviewer.html#?job_id=76315&repo=mozilla-b2g37_v2_2
Flags: needinfo?(thills)
Ugh, I obviously can't read. I have no idea why I thought bug 1136664 was going to help here.
No longer depends on: 1136664
Flags: needinfo?(dale)
(Reporter)

Comment 18

4 years ago
Hi Tamara, 

Can we uplift this to 2.2?
So somehow a line that was removed in master did not get removed in 2.2

https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15 is the master commit, https://github.com/mozilla-b2g/gaia/commit/df92a3c0d8ed757c04b81085c260da9234c59981 is the 2.2 one

The |SearchProvider.ready()| was removed in master but not in the 2.2 commit, a little confused as to how that can happen since it wasnt a branch patch but I am not a git expert, Ryan do you have any ideas?

Tamara is gonna do a branch patch to remove that and hopefully the existing test should work as is.
Flags: needinfo?(dale)
No clue, sorry.
(Assignee)

Comment 22

4 years ago
Comment on attachment 8582392 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search-2.2-branch-patch > mozilla-b2g:v2.2

Hi Marshall,

I talked with Dale and we decided best approach is to make a branch patch.  Essentially, there was a merge conflict from bug 1131966 where a merge didn't get correctly resolved on the uplift.  So, we decided I should remove this line in the branch patch.  

BTW, I'm not sure what the process is for branch patch review, so just adding a review flag on this.  It's the same patch as the 3.0, except with this line removed: https://github.com/mozilla-b2g/gaia/pull/29097/files#diff-a72719dbedcb33e670a082ee4341d876L36

For reference here is the merge commit difference on bug 1131966
master commit:  https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15

2.2 commit:  https://github.com/mozilla-b2g/gaia/commit/df92a3c0d8ed757c04b81085c260da9234c59981
Flags: needinfo?(thills)
Attachment #8582392 - Flags: review?(marshall)
Comment on attachment 8582392 [details] [review]
[gaia] tamarahills:bugfix/1141891-add-suggestion-search-2.2-branch-patch > mozilla-b2g:v2.2

Looks good, just had one quick question about a removed line of code
Attachment #8582392 - Flags: review?(marshall) → review+
QA Whiteboard: [QAExclude]
Flags: needinfo?(jmercado)
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.