Closed
Bug 390295
Opened 18 years ago
Closed 17 years ago
Searchplugin with digit in url cuts off url
Categories
(SeaMonkey :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: linuxbox, Assigned: bugzilla)
Details
(Keywords: fixed-seamonkey1.1.10)
Attachments
(1 file, 1 obsolete file)
1.12 KB,
patch
|
neil
:
review+
neil
:
superreview+
kairo
:
approval-seamonkey1.1.10+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.8.1.5) Gecko/20070716 SeaMonkey/1.1.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.8.1.5) Gecko/20070716 SeaMonkey/1.1.3
I have a metager2.src with the content below.
If yout hit the search button of the empty addressbar to call the searchsite the url
http://www.metager2.de/search.php is cut off to http://www.metager/
###################################################################
<search
version="1.0"
name="MetaGer2"
description="MetaGer2 Meta-Suchmaschine"
method="GET"
action="http://www.metager2.de/search.php"
update="http://www.metager2.de/mozilla/metager2.src"
updateCheckDays=1
queryEncoding="iso-8859-1"
queryCharset="iso-8859-1"
>
<input name="q" user>
</search>
<browser
update="http://www.metager2.de/mozilla/metager2.src"
updateIcon="http://www.metager2.de/mozilla/metager2.gif"
updateCheckDays="3"
>
###################################################################
Reproducible: Always
Steps to Reproduce:
1. Install the searchplugin http://www.metager2.de/mozilla.php
2. Make the searchplugin metager2 the default searchengine in preferences
3. Hit the browser searchbutton with an empty addressbar
Actual Results:
The URL ist cut off to http://metager/
The sidebar search works.
Reporter | ||
Comment 1•18 years ago
|
||
This patch solves the problem:
xpfe/browser/resources/content/navigator.js
# diff navigator.js navigator.js.DIST
1239c1239
< searchRoot = searchURL.match(/[a-z]+:\/\/[a-z.-]+/);
---
> searchRoot = searchURL.match(/[a-z]+:\/\/[a-z0-9.-]+/);
Comment 2•17 years ago
|
||
Klaus, please attach your patch to bug and request review for it from respective module owner.
Reporter | ||
Comment 3•17 years ago
|
||
Hi Misak,
the bug still exist with SeaMonkey 1.1.8. I don't know what do more than reporting the bug here. I also don't know a module owner, because the code is in a general component. The simple patch is posted above, so what should I attach more?
Bye Klaus
Comment 4•17 years ago
|
||
Hi Klaus,
Here is the two useful links for you, if you have time of course:
http://www.mozilla.org/hacking/
http://www.mozilla.org/owners.html
Also you can visit IRC channel #seamonkey at irc.mozilla.org and ask developers what to do with patch. I'm sure, You'll find help there.
Comment 5•17 years ago
|
||
Here another link - http://www.seamonkey-project.org/dev/project-areas
Seems Mnyromir is who you need. You can catch him on irc
Reporter | ||
Comment 6•17 years ago
|
||
Hi Misak,
I think it should be enough for a "normal" user to file a bug report, as I'm not a developer. As far as I know this is the right place for mozilla bug reports. I also think that the developers search and work on the bug reports, so why should I bother them on IRC? The subject of this bug report is clear and the right developer should be able to find this report.
But you are always welcome to do anything with this bug report and the patch :-)
IMHO we should stop, filling this bugreport with general discussions about the handling of mozilla bug reporting. If you furthermore wish to discuss this, feel free to send me a private mail.
Klaus
Comment 7•17 years ago
|
||
Ok Klaus, I get your point. I'll try to poke Mnyromir to look at your patch.
I just doing some cleanup of bugs, that don't have activity long time, and want make Seamonkey better. With your patch, it should be better a bit. Thanks and sorry.
Assignee | ||
Comment 8•17 years ago
|
||
The solution suggested by Klaus in comment#1 matches any alphadigit combinations as part of searchRoot (like metager2.de and also pure IP addresses).
The patch I made is trunk only, don't know if we want it for branch.
Assignee: general → aqualon
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #312045 -
Flags: review?(mnyromyr)
Comment 9•17 years ago
|
||
Comment on attachment 312045 [details] [diff] [review]
Support URLs with digits and IP addresses as searchRoot
> // Get a search URL and guess that the front page of the site has a search form.
> var searchDS = Components.classes["@mozilla.org/rdf/datasource;1?name=internetsearch"]
> .getService(Components.interfaces.nsIInternetSearchService);
> var searchURL = searchDS.GetInternetSearchURL(searchEngineURI, "ABC", 0, 0, {value:0});
> if (searchURL) {
>- searchRoot = searchURL.match(/[a-z]+:\/\/[a-z.-]+/);
>+ searchRoot = searchURL.match(/[a-z]+:\/\/[a-z0-9.-]+/);
> if (searchRoot) {
> openTopWin(searchRoot + "/");
> return;
> }
My goodness, this code is just evil guessing! Any IDN or uppercase letters and it's doomed. And it won't work with arbitrary protocols, either.
A slightly better regex, imo, would be this:
/^[a-z-]+:\/\/[a-z0-9.-]+/i
This ties the match to the beginning of the string, allows protocols like our internal mailbox-message:// and ignores the character case. IDN still won't work, but they can be made working by replacing them with their punycode.
r=me with that.
Attachment #312045 -
Flags: review?(mnyromyr) → review+
Assignee | ||
Comment 10•17 years ago
|
||
How about one of those suggestions:
/* matching all between :// and / */
/^[a-z-]+:\/\/[^/]*/
/* matches additional all kind of characters in the protocol part */
/^[^]+:\/\/[^/]*/
I think the first one would be sufficient, but I don't know very much about regex.
Assignee | ||
Comment 11•17 years ago
|
||
Neil: Mnyromyr suggested the final regex, so asking you for review :)
Attachment #312045 -
Attachment is obsolete: true
Attachment #312381 -
Flags: superreview?(neil)
Attachment #312381 -
Flags: review?(neil)
Comment 12•17 years ago
|
||
Comment on attachment 312381 [details] [diff] [review]
allow schema://host as searchRoot
bz or biesi would probably prefer this to go via the IO service but I'm sure it's good enough for what we need.
Attachment #312381 -
Flags: superreview?(neil)
Attachment #312381 -
Flags: superreview+
Attachment #312381 -
Flags: review?(neil)
Attachment #312381 -
Flags: review+
Comment 13•17 years ago
|
||
Checking in navigator.js;
/cvsroot/mozilla/suite/browser/navigator.js,v <-- navigator.js
new revision: 1.619; previous revision: 1.618
done
Assignee | ||
Comment 14•17 years ago
|
||
Comment on attachment 312381 [details] [diff] [review]
allow schema://host as searchRoot
Do we want this in branch?
Pro:
Only a small change to allow sites like metager2.de.
Contra:
The only use case is clicking on the search button with no userinput in the urlbar.
Attachment #312381 -
Flags: approval-seamonkey1.1.10?
Reporter | ||
Comment 15•17 years ago
|
||
This are two pro's:
Only a small change *and* it has a use case :-)
Thanks for the changes!
Bye Klaus
![]() |
||
Comment 16•17 years ago
|
||
Comment on attachment 312381 [details] [diff] [review]
allow schema://host as searchRoot
From what I see, this fixes at least one known broken case, has no security risk, doesn't break any currently working cases, and seems to have no substantial risk to cause problems, so everything clear for a branch fix ;-)
a=me for 1.1.10 (1.8 branch)
Attachment #312381 -
Flags: approval-seamonkey1.1.10? → approval-seamonkey1.1.10+
![]() |
||
Comment 17•17 years ago
|
||
This has been checked in by mcsmurf on trunk (comment #13), and I just checked it into 1.8 branch per Aqaualon's request on IRC.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: fixed-seamonkey1.1.10
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•