390295 - Searchplugin with digit in url cuts off url

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Klaus Fuerstberger]

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.

Comment 1

[Klaus Fuerstberger]

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

[Misak Khachatryan]

Klaus, please attach your patch to bug and request review for it from respective module owner.

Comment 3

[Klaus Fuerstberger]

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

[Misak Khachatryan]

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

[Misak Khachatryan]

Here another link - http://www.seamonkey-project.org/dev/project-areas
Seems Mnyromir is who you need. You can catch him on irc

Comment 6

[Klaus Fuerstberger]

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

[Misak Khachatryan]

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.

Comment 8

[Bruno 'Aqualon' Escherl]

Attachment: Support URLs with digits and IP addresses as searchRoot

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.

Comment 9

[Karsten Düsterloh]

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.

Comment 10

[Bruno 'Aqualon' Escherl]

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.

Comment 11

[Bruno 'Aqualon' Escherl]

Attachment: allow schema://host as searchRoot

Neil: Mnyromyr suggested the final regex, so asking you for review :)

Comment 12

[[e-mail]]

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.

Comment 13

[Frank Wein [:mcsmurf]]

Checking in navigator.js;
/cvsroot/mozilla/suite/browser/navigator.js,v  <--  navigator.js
new revision: 1.619; previous revision: 1.618
done

Comment 14

[Bruno 'Aqualon' Escherl]

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.

Comment 15

[Klaus Fuerstberger]

This are two pro's:

Only a small change *and* it has a use case :-)

Thanks for the changes!

Bye Klaus

Comment 16

[Robert Kaiser]

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)

Comment 17

[Robert Kaiser]

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.