Closed
Bug 427155
Opened 16 years ago
Closed 16 years ago
searching for /find bar/ under "get add-ons" generates error
Categories
(Toolkit :: Add-ons Manager, defect, P2)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b2
People
(Reporter: mcdavis941.bugs, Assigned: mossop)
References
Details
Attachments
(1 file)
1.82 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
Searching for the /find bar/ add-on in the Add-on Manager's "Get Add-ons" pane generates three of this error: document.getElementById(id) is null chrome://mozapps/content/extensions/extension.xml line 387 STR: 1. Use 20080404 minefield nightly 2. Open Add-ons Mgr to "Get Add-ons" pane 3. Enter /find bar/ in the search box and search for it 4. Errors show up in console Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040404 Minefield/3.0pre
Assignee | ||
Comment 1•16 years ago
|
||
The error message in the console is unrelated. I think this is related to bug 415269 and may be an AMO issue. The server seems to be unescaping the escaped search terms somewhere along the way leading to an invalid query: https://services.addons.mozilla.org/en-US/firefox/api/1/search/%2ffind%20bar%2f Any ideas Laura?
Comment 2•16 years ago
|
||
I'm going to pile on a few more strangenesses and inconsistencies. Right now if you do a search for any of the following chars: !#&-_+: You'll get some results or "no match" If your search includes any of the following characters, you'll get a "Firefox couldn't retrieve add-ons": ~`@$%^*()=[]\{}|;'",/<>? We need to be consistent, I thought that the error message was only displayed for connectivity issues. Is the matching due to some regex/XML input filtering? Is this Add-on Mgr code or AMO API? Also, searching for "-" or "." gives me MANY matches but no results in the list, is that expected behavior?
Assignee | ||
Comment 3•16 years ago
|
||
(In reply to comment #2) > I'm going to pile on a few more strangenesses and inconsistencies. Right now if > you do a search for any of the following chars: !#&-_+: > > You'll get some results or "no match" That is exactly what the API tells us: https://services.addons.mozilla.org/en-US/firefox/api/1/search/test!&-_+ > If your search includes any of the following characters, you'll get a "Firefox > couldn't retrieve add-ons": ~`@$%^*()=[]\{}|;'",/<>? For these characters the API is returning a completely empty page, no XML to process: https://services.addons.mozilla.org/en-US/firefox/api/1/search/@find > We need to be consistent, I thought that the error message was only displayed > for connectivity issues. Is the matching due to some regex/XML input filtering? > Is this Add-on Mgr code or AMO API? The addons manager is behaving as sensibly as possible in this regard I believe. It displays the error when the server returns an invalid response, which means any error pages or a result that isn't XML. > Also, searching for "-" or "." gives me MANY matches but no results in the > list, is that expected behavior? Some of the changes that were made to the API to improve performance involved only including the first 10 search results in the list returned to the client. In these cases it looks like there are indeed a lot of matches, but none of the first 10 are 3.0 compatible. It is I agree not an ideal behaviour, it should improve with bug 419057 https://services.addons.mozilla.org/en-US/firefox/api/1.1/search/-
Updated•16 years ago
|
Product: Firefox → Toolkit
Assignee | ||
Comment 5•16 years ago
|
||
The bits from comment 2 and 3 seem fixed now. The original issue still exists and it is the API giving an error response so I believe this is an API issue.
Component: Add-ons Manager → API
Product: Toolkit → addons.mozilla.org
QA Contact: extension.manager → api
Version: Trunk → 3.2
Updated•16 years ago
|
Assignee: nobody → laura
Target Milestone: --- → 4.0.2
Updated•16 years ago
|
Priority: -- → P2
Comment 6•16 years ago
|
||
Ok, so after much research and banging my head against mod_rewrite, I finally found this is actually caused by an Apache bug. https://issues.apache.org/bugzilla/show_bug.cgi?id=34602 We can't do much about it at the API level, since Apache chokes on the URL and it never gets any further. There is a workaround here: http://us.php.net/manual/en/function.urlencode.php#73641 basically, double urlencode params that contain special characters. This will have to be worked around at the Addons Manager level, re-assigning.
Assignee: laura → nobody
Component: API → Add-ons Manager
OS: Windows XP → All
Product: addons.mozilla.org → Toolkit
QA Contact: api → extension.manager
Hardware: PC → All
Target Milestone: 4.0.2 → ---
Version: 3.2 → unspecified
Assignee | ||
Comment 7•16 years ago
|
||
Will it require an API change to unencode the url twice first?
Comment 8•16 years ago
|
||
I'll test, but I don't think so. (If it's going to cause a problem we'll have to do an API version bump, so I hope not.)
Assignee | ||
Comment 9•16 years ago
|
||
It looks as though it is working fine with just the client side change. However given that we're effectively changing the url passed for a given search we may have to consider revving the api version anyway in case people have been pointing to other services. I haven't heard of this happening so potentially we could ignore it, but imagine the case where apache got fixed, and AMO updated, you'd then be in a position where urls from Firefox 3.1 were encoded too much
Assignee | ||
Comment 10•16 years ago
|
||
Ok I have increased the API version to 1.2 with this change. AMO already seems to be happy dishing out results on that but can you confirm you are happy with the bump Laura?
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attachment #343198 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•16 years ago
|
Attachment #343198 -
Flags: review?(laura)
Updated•16 years ago
|
Attachment #343198 -
Flags: review?(robert.bugzilla) → review+
Comment 11•16 years ago
|
||
If you tested this, and it works as is vs servieces.AMO ...the question is if we actually need the version bump?
Comment 12•16 years ago
|
||
That would be "services" as in https://services.addons.mozilla.org. Please excuse my typing.
Assignee | ||
Comment 13•16 years ago
|
||
(In reply to comment #11) > If you tested this, and it works as is vs servieces.AMO ...the question is if > we actually need the version bump? I am saying we do because we have changed the url the client requests. While it may not make much difference to AMO, it may do to the API service that say Songbird might choose to use. It could be pretty critical for them to know how escaped the request url is.
Comment 14•16 years ago
|
||
OK cool - as long as everybody realizes the code doesn't change at all on the API side (how confusing :) )
Assignee | ||
Updated•16 years ago
|
Attachment #343198 -
Flags: review?(laura)
Assignee | ||
Comment 15•16 years ago
|
||
Landed http://hg.mozilla.org/mozilla-central/rev/60cadfb3a231
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Comment 16•15 years ago
|
||
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3pre) Gecko/20090819 Shiretoko/3.5.3pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3pre) Gecko/20090819 Shiretoko/3.5.3pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•