In bug 326532, in addition to &, a number of other characters apparently got encoded. Encoding / breaks a number of my keywords (Firefox does not encode /, and at least Google searches for items with / in them work properly with / not encoded).
Any idea how the fix for bug 295991 (a fix for encoding problems with keyword search at the docshell level) affects this (and the post-bug 326532 encoding situation in general)?
It seems better in general to assume that user-input should be escaped when it overlaps with reserved characters, as they are not likely to be using them in their reserved meanings--and doing too much escaping is generally fine for URLs. Could you give an example of a keyword that breaks because of encoding / ?
STR: 1. Make a keyworded bookmark to http://wiki.caminobrowser.org/Main_Page/%s 2. invoke the bookmark with a %s of Development:Reviewing What happens: http://wiki.caminobrowser.org/Main_Page/Development%3AReviewing is loaded What should happen: http://wiki.caminobrowser.org/Main_Page/Development:Reviewing is loaded
(obviously, based on the original report, the same keyword will also break with the / for something "User:Sardisson/Dogfood")
(In reply to comment #3) > STR: > > 1. Make a keyworded bookmark to http://wiki.caminobrowser.org/Main_Page/%s > 2. invoke the bookmark with a %s of Development:Reviewing > > What happens: http://wiki.caminobrowser.org/Main_Page/Development%3AReviewing > is loaded > What should happen: > http://wiki.caminobrowser.org/Main_Page/Development:Reviewing is loaded Those are equivalent pages; I'm not seeing the bug. (In reply to comment #4) > (obviously, based on the original report, the same keyword will also break with > the / for something "User:Sardisson/Dogfood") If the page is named with a slash, it needs to be escaped in the URL, so that's not a valid case. Unless you are suggesting that users should be expected to construct valid URL fragments--in which case & and = shouldn't be escaped either in case I want to do something like do a Google search for "foo&btnI=I'm Feeling Lucky" when I want the top result.
I guess with the increasing popularity of RESTful URLs, I can see an argument for leaving '/' alone, and it should be fine in query strings according the RFC. I think the other characters should stay escaped though, unless there are any others with compelling reasons not escape.
Sorry, my examples should have all had the Main_Page/ removed from them (including the keyword). Think before typing, think before typing. /me takes a breather
And yes, I realize now that the ":" encoding doesn't make a difference. Maybe I'm doing more harm than good here... (sorry for the bugspam)
Does what it says on the package. Consider this atonement for every other comment I've made in this bug ;)
Assignee: nobody → stridey
Status: NEW → ASSIGNED
Attachment #228608 - Flags: review?(hwaara)
Comment on attachment 228608 [details] [diff] [review] Doesn't Encode / r=me, moving hwaara to second-r, asking for sr from pink.
Comment on attachment 228608 [details] [diff] [review] Doesn't Encode / sure, if everyone's happy with this. rs=pink
Attachment #228608 - Flags: superreview?(mikepinkerton) → superreview+
Baking on trunk and branch
You need to log in before you can comment on or make changes to this bug.