260794 - Duplicate parameter generated in Google search

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Ng Ming Hong]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040913 Firefox/0.10

I find that the parameter 'start' (for Google search) generated by Search
Toolbar is duplicated.

e.g. If I search for the keyword 'Meo':
http://www.google.com/search?q=Meo&start=0&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official
I think this URL is good enough:
http://www.google.com/search?q=Meo&&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official

Reproducible: Always
Steps to Reproduce:
1. Focus on the Search Toolbar.
2. Input any keyword and press [Enter].
3. Observe the URL generated.

Actual Results:  
Give me a duplicate parameter: 'start=0'

Expected Results:  
Each parameter should appear only once.

Comment 1

[Ng Ming Hong]

Typo, should be:
http://www.google.com/search?q=Meo&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official
Instead of:
http://www.google.com/search?q=Meo&&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official

Comment 2

[Arthur Wiebe]

Reproduced on:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; rv:1.7.3) Gecko/20040913
Firefox/0.10

The severity is too high since there is no loss of function. Should be set to minor.

Comment 3

[Ng Ming Hong]

Changed to minor.
But I believe it just need an ultra simple fix...

Comment 4

[Arthur Wiebe]

I already have the fix. Just making sure this actually is a bug :)

Comment 5

[Arthur Wiebe]

Attachment: Patch

Are there any patch guidelines?

Comment 6

[Arthur Wiebe]

Attachment: [duplicate] Patch

Comment 7

[Arthur Wiebe]

Might as well change status to new.
Sorry about the spam.

Comment 8

[Arthur Wiebe]

Comment on attachment 159601 [details] [diff] [review]
Patch

I set the wrong flag before.

Comment 9

[Asa Dotzler [:asa]]

Ben, or mconnor, if this is an easy fix, can you review or land it? 

Comment 10

[Ng Ming Hong]

Just as a remainder, this bug is still reproducible in Deer Park Alpha 2 (not
surprisingly).

Comment 11

[Mike Connor [:mconnor]]

We should fix this, and get google to update their updated .src file (once I fix
the updating searchplugins bug)

But that still doesn't make this a blocker.

Comment 12

[Wayne Mery (:wsmwk)]

Comment on attachment 159601 [details] [diff] [review]
Patch

request Ben to review (trivial)

Comment 13

[Phil Ringnalda (:philor)]

Attachment: fix without breaking things

There's no need to break things for either the SearchSidebar extension, or a
SeaMonkey user who somehow gets our version: when you have a single param for
previous and next, "<inputprev>" says "use the name and factor from inputnext,
but decrement it." Verified working with SeaMonkey, and working within the
limits of some bugs in SearchSidebar.

There's also no need to wait on Google changing their version: we're shipping a
plugin that claims to update from mozilla.org, and bug 306576 is going to do it
for real from addons.m.o, so there's no coordination requirement.

Comment 14

[Phil Ringnalda (:philor)]

Comment on attachment 196621 [details] [diff] [review]
fix without breaking things

Asking for approval without being quite sure we want it: if bug 306576 is going
to put actual content at the update urls on addons.m.o, not just do our current
404, then if we don't land this on the branch they'll get a nice bit of testing
as everyone who installs 1.5 then updates their google.src first check...

Comment 15

[Mike Schroepfer]

Comment on attachment 196621 [details] [diff] [review]
fix without breaking things

Approved for 1.8b5 per bug meeting

Comment 16

[Asaf Romano (gone)]

Trunk:
Checking in browser/locales/en-US/searchplugins/google.src;
/cvsroot/mozilla/browser/locales/en-US/searchplugins/google.src,v  <--  google.src
new revision: 1.4; previous revision: 1.3
done

1.8 Branch:
Checking in browser/locales/en-US/searchplugins/google.src;
/cvsroot/mozilla/browser/locales/en-US/searchplugins/google.src,v  <--  google.src
new revision: 1.3.2.1; previous revision: 1.3
done

Comment 17

[Axel Hecht [:Pike]]

A message to localizers, you probably want to take this patch for your 1.5
localizations.
As it's just a nit, I won't approve this change for the 1.0.x branch, though.

Is someone on this bug willing to take the burdon to fix the plethora of plugins
on mycroft?

A note on the google-hosted updateURL, we shouldn't put content there, as most
localizers used the same updateURL which slipped review on 1.0.x. At least we
shouldn't do that for this nit.