278919 - new search feature is crazy broken

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[(not currently active) Ted Mielczarek]

Yeah.  Something goes terribly wrong with certain searches.

I'm on it.

Comment 1

[alanjstr]

Search results for Pinball says I'm on page 2 of 2, results 11-12.  There's
nothing on the page.  I also would only expect 1 result (or very few).  We're
looking for a simple search and simple results.  This should only be pulling
from the main table (for now)

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: Fixes some stuff

Yeah, not enough testing on my part.  Also the style sucks less now.

Comment 3

[alanjstr]

Comment on attachment 171711 [details] [diff] [review]
Fixes some stuff

I don't like that the page header is an h3 and the names are h2s.  

$pageid = $_GET["pageid"];
And then when you go to use it later, you're not escaping it.  Nor are you
making sure it is an integer.

if ($_GET["q"] || $_GET["pageid"]) $didSearch = 1;

If we don't have a q, then how did we do a search?  

$typedirs = array("E"=>"extensions","T"=>"themes","U"=>"update");
Move that outside of the loop

General issue:
are we sure the content inside the database is html safe (trustable)?

Comment 4

[(not currently active) Ted Mielczarek]

Attachment: Fixes the issues.

Ok.  "Results" is in a h1 now.	$didSearch only gets set if $q is set.	$pageid
is ensured integer.  I removed #mainContent and it seems to look better. 
$typedirs moved outside the array.  The search results are h2, and extension
names and descriptions get escaped with htmlspecialchars().

Comment 5

[alanjstr]

Comment on attachment 171715 [details] [diff] [review]
Fixes the issues.

Great!	Thanks.

Comment 6

[Anne (:annevk)]

Things like "&section" within the HREF attribute should be encoded as
"&section". Also, instead of using « I think we should use the actual
character. Since update is using UTF-8 that is possible. Why is the  
exactly needed?

Also, most files have a newline at the end. Not sure how update does that though.

Comment 7

[(not currently active) Ted Mielczarek]

Oops, that's my bad on the & missing.  Should have caught that.  I have no
idea how to actually insert a Unicode character for what we currently have as
entities.  Is it really important, or just good practice, or what?

As for the  s, I don't really know why they're there.  If they bother you
we could take them out.

Comment 8

[alanjstr]

You know how we have that big gap on the left?  How about a simple box that
links to Home like https://addons.update.mozilla.org/plugins/

Comment 9

[(not currently active) Ted Mielczarek]

Attachment: Escape those ampersands

I didn't do anything about   or the Unicode characters.

Comment 10

[alanjstr]

Comment on attachment 171816 [details] [diff] [review]
Escape those ampersands

Why are we getting the page id if didSearch !=1.

Comment 11

[alanjstr]

Fix checked in on branch and trunk by mconnor