Closed Bug 1159717 Opened 5 years ago Closed 4 years ago

Loading the search form from an empty search bar doesn't send the purpose

Categories

(Firefox :: Search, defect)

defect
Not set
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.1 - May 25
Tracking Status
firefox39 --- verified
firefox40 --- verified
firefox41 --- verified

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
I noticed this while looking into bug 1131274. The search form url is obtained from a 'searchForm' getter on the engine, so the purpose argument isn't provided.

Only requesting feedback for now, as this probably deserves a test.
Attachment #8599297 - Flags: feedback?(gavin.sharp)
Comment on attachment 8599297 [details] [diff] [review]
Patch

Function name should be _getSearchFormWithPurpose to indicate that it's not part of the public API.

Thanks for catching this.
Attachment #8599297 - Flags: feedback?(gavin.sharp) → feedback+
Attached patch Patch v2Splinter Review
Assignee: nobody → florian
Attachment #8599297 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8608143 - Flags: review?(markh)
Attachment #8608143 - Flags: review?(markh) → review+
Steps to verify:
- Click on the glass icon while the searchbox is empty.
- Click on the Yahoo icon.

The url loaded should contain: hsimp=yhs-001 (We load https://search.yahoo.com/yhs/search?p=&ei=UTF-8&hspart=mozilla&hsimp=yhs-001 and that redirects me to https://search.yahoo.com/yhs/?hspart=mozilla&hsimp=yhs-001)

Without the patch, we load https://search.yahoo.com/yhs/search?p=&ei=UTF-8&hspart=mozilla which redirects me to https://search.yahoo.com/web?fr=yhs-invalid

I only gave steps to verify for the yahoo plugin, but this patch will also change the URLs used for Bing, and for DuckDuckGo (with bug 1131274 fixed).
Points: --- → 3
Flags: qe-verify+
Flags: firefox-backlog+
Comment on attachment 8608143 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: Search
[User impact if declined]: Users would be sent to an incorrect Yahoo URL, and consequently have a different search experience.
[Describe test coverage new/current, TreeHerder]: the patch contains tests
[Risks and why]: low risks; affects only the URLs used when the searchbox is empty.
[String/UUID change made/needed]: none.
Attachment #8608143 - Flags: approval-mozilla-beta?
Attachment #8608143 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/bcb6aef77b95
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8608143 [details] [diff] [review]
Patch v2

Early in the beta cycle, search related. Taking it.
Attachment #8608143 - Flags: approval-mozilla-beta?
Attachment #8608143 - Flags: approval-mozilla-beta+
Attachment #8608143 - Flags: approval-mozilla-aurora?
Attachment #8608143 - Flags: approval-mozilla-aurora+
Iteration: --- → 41.1 - May 25
Verified that empty Yahoo searches from Search toolbar are redirected to the correct url using Firefox 39 beta 1 build2, latest Aurora 40.0a2 and latest Nightly 41.0a1 2015-05-25 under Win 7 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.9.5.

Florian, I've noticed that under Win and Ubuntu the https://search.yahoo.com/yhs/?hspart=mozilla&hsimp=yhs-001 has a smaller Yahoo! logo than https://search.yahoo.com/ (please see http://i.imgur.com/rgGfphD.png) and I'm wondering if it should be logged on bugzilla. Thank you!
Status: RESOLVED → VERIFIED
Flags: needinfo?(florian)
(In reply to Petruta Rasa [QA] [:petruta] from comment #12)

> Florian, I've noticed that under Win and Ubuntu the
> https://search.yahoo.com/yhs/?hspart=mozilla&hsimp=yhs-001 has a smaller
> Yahoo! logo than https://search.yahoo.com/ (please see
> http://i.imgur.com/rgGfphD.png) and I'm wondering if it should be logged on
> bugzilla. Thank you!

This is actually a question for kev.
Flags: needinfo?(florian) → needinfo?(kev)
Flags: needinfo?(kev)
You need to log in before you can comment on or make changes to this bug.