Closed Bug 925892 Opened 6 years ago Closed 6 years ago

add "channel=sb" parameter to Google plugin to distinguish search bar searches

Categories

(Firefox :: Search, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 27

People

(Reporter: Gavin, Assigned: Gavin)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently search bar searches cannot easily be distinguished from the others, apart from "lack of a 'channel' param". It would be better for that to be more explicit.
Attached patch patch (obsolete) — Splinter Review
This adds a channel=sb parameter to searches triggered by our search bar.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Summary: add "channel=searchbar" parameter to Google plugin to distinguish search bar searches → add "channel=sb" parameter to Google plugin to distinguish search bar searches
Comment on attachment 816046 [details] [diff] [review]
patch

Review of attachment 816046 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/searchplugins/google.xml
@@ +25,5 @@
>  #endif
>    <MozParam name="channel" condition="purpose" purpose="contextmenu" value="rcs"/>
>    <MozParam name="channel" condition="purpose" purpose="keyword" value="fflb"/>
>    <MozParam name="channel" condition="purpose" purpose="homepage" value="np"/>
> +  <MozParam name="channel" condition="purpose" purpose="searchbar" value="sb"/>

Nit: s/searchbar/searchbox/ throughout the changes.
Attachment #816046 - Flags: feedback+
Attached patch patchSplinter Review
Attachment #816046 - Attachment is obsolete: true
Attachment #816110 - Flags: review?(mnoorenberghe+bmo)
Comment on attachment 816110 [details] [diff] [review]
patch

Review of attachment 816110 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry about changing my mind but after seeing that FHR is already using "searchbar" and it's also the id of the binding I guess we should be consistent. r=me with the change.

While reviewing this patch, I found that the command line search argument is a remaining getSubmission call without a purpose argument. I filed bug 925967 to add a purpose and start warning when one is missing.
Attachment #816110 - Flags: review?(mnoorenberghe+bmo) → review+
I don't think it makes sense to send the extra seven characters on billions of requests to be consistent with our internal code.  =sb is what we should use in this case, unless Google says otherwise.
(In reply to Mike Connor [:mconnor] from comment #6)
> I don't think it makes sense to send the extra seven characters on billions
> of requests to be consistent with our internal code.  =sb is what we should
> use in this case, unless Google says otherwise.

"sb" is what gets sent in the URL. The internal purpose argument maps to a value provided by the search engine. See the snippet in comment 2.
Joanne, can you confirm that we're clear to make this change on Nightly (i.e. that it won't cause any trouble on Google's end)?
Flags: needinfo?(jnagel)
I have shared the information with Google and just waiting for confirmation that this will not be an issue on their end.  Thanks, Joanne
Flags: needinfo?(jnagel)
Hi Gavin, Google's technical team has confirmed the channel "sb".  Once you have a timeline that I can share with them, could you let me know?  Thanks, Joanne
https://hg.mozilla.org/mozilla-central/rev/19b5c3b11c94
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 975528
You need to log in before you can comment on or make changes to this bug.