Closed Bug 390295 Opened 18 years ago Closed 17 years ago

Searchplugin with digit in url cuts off url

Categories

(SeaMonkey :: General, defect)

x86
Linux
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: linuxbox, Assigned: bugzilla)

Details

(Keywords: fixed-seamonkey1.1.10)

Attachments

(1 file, 1 obsolete file)

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.
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.-]+/);
Klaus, please attach your patch to bug and request review for it from respective module owner.
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
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.
Here another link - http://www.seamonkey-project.org/dev/project-areas Seems Mnyromir is who you need. You can catch him on irc
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
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.
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 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+
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.
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 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+
Checking in navigator.js; /cvsroot/mozilla/suite/browser/navigator.js,v <-- navigator.js new revision: 1.619; previous revision: 1.618 done
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?
This are two pro's: Only a small change *and* it has a use case :-) Thanks for the changes! Bye Klaus
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+
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
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: