Closed Bug 750668 Opened 12 years ago Closed 12 years ago

Search input not properly url encoded when sent to search server

Categories

(Pancake Graveyard :: Front-end, defect)

Product:
Pancake Graveyard
Component:
Front-end
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: deb, Assigned: sfoster)

Details

I just did a search for "B & H Photo" which returned results for just "B", "&", "H" and "Photo".  Obviously this is suboptimal because the top result was for the "B" wikipedia page, and I ended up with a bunch of unrelated social results based on "photo".

Could we possibly restrict searches to the full string, rather than just chunks?  I'm not sure what the reasoning behind this is, so maybe I'm missing something.
Assignee: nobody → oyiptong
Target Milestone: --- → M3
This is due to the Front End not urlencoding the  textbox in pancake.

the search query "b & h photo" results in an api call with a query of "/search/bing?q=b+&format=json" in the backend.

Anything after and including the ampersand symbol has been truncated.
Component: General → Front-end
QA Contact: general → frontend
Assignee: oyiptong → nobody
The expected backend search query is:

/search/bing?q=b+%26+h+photo&format=json
Summary: Search returns results for all individual words, not just full string → Search input not properly url encoded when sent to search server
Assignee: nobody → sfoster
Turns out some browsers (including Firefox and UiWebView) decode location.hash, others do not. This is a Backbone bug, fixed in the latest version: https://github.com/documentcloud/backbone/commit/07604deac0b6e00492d62b9523c19f74e5bd7542

This is just part of the problem, but we can't fix the whole problem without addressing this. I'll start a branch with the new backbone.js and test it out to see if we can update it fairly cleanly
input: 'b & h photo' now results in a request for
http://colin.local:6543/search/bing?format=json&q=b+%26+h+photo

..plus a bunch of other cases unit tested for. There's still a couple of failing tests highlighting the fact we can't round trip strings through encodeURIComponent -> decodeURIComponent. In practice this is ok, so I may either fix or remove those tests. 

I ended up just patching our Backbone.js with the relevant bit so this issue doesn't get conflated with any other backbone upgrade issues.  

Fixed in https://bitbucket.org/mozillapancake/pancake/changeset/4983bbd3cc60
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.