79655 - Internet Keywords: bypassed if "." or ":" appears in user input

Status: RESOLVED DUPLICATE of bug 261608

(SeaMonkey :: Search, defect)

Description

[zane]

What I did:
------------
I typed the following: "ide-scsi modules.conf linux" (without the double quotes)
into my location bar, and hit return.

What happened:
----------------
I got an error message saying "ide-scsi modules.conf linux could not be found,
please check the name and try again".  If I remove the '.' from 'modules.conf' a
search is run, so I'm guessing you're using the period as an indicator that the
user has entered what they believe to be a hostname.

I also tried double quoting the string '"modules.conf"' hoping that it would
cause it to ignore the period, but this did not work.  Would be a nice
enhancement though.

What I expected:
------------------
I expected a search on google for those strings to be run, and the results
returned.  All in all, I guess I would just prefer that I could more easily
control the behaviour of the location bar - i.e. what it does when I give it
input of different kinds.

Comment 1

[Dave]

Confirming.
Maybe mozilla should search for spaces as well as a "." and run a search if
there're spaces in the URL?

Comment 2

[Dave]

This is actually a dupe of bug 73974 which is marked fixed...

*** This bug has been marked as a duplicate of 73974 ***

Comment 3

[matt]

this is not a dup of that bug.
That bug is when a user hits the search button.
This bug is enter.  It has worked like this for years including 4.7.

Comment 4

[Judson Valeski]

looks like we might need to do the space check before the dot check. this get's
tricky though because we might try to handle directories w/ spaces in them like:

file://Program Files or http://foo/my dir

Comment 5

[timeless]

the http version isn't valid.  The file:// one is a problem.

Comment 6

[Samir Gehani]

Using internet keywords might help.  Enable it from the
Edit|Preferences|Navigator|SmartBrowsing pref panel.

Comment 7

[R.K.Aa.]

*** Bug 114892 has been marked as a duplicate of this bug. ***

Comment 8

[R.K.Aa.]

*** Bug 115139 has been marked as a duplicate of this bug. ***

Comment 9

[m_mozilla]

the following fix is from bug 115139


http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#957 wrote:
957> PRInt32 dotLoc = hostStr.FindChar('.');
 :
994> if(keywordsEnabled && (-1 == dotLoc)) {


The previous line 994 becomes the following two lines:


994> PRInt32 spaceLoc = hostStr.FindChar(' ');
995> if(  keywordsEnabled  && ( (spaceLoc > -1) || (-1 == dotLoc) )  ) {


that gives us the desired search functionality.

regarding Comment #4 and Comment #5 about file URLs. If I'm reading the code
correctly, this is already handled in lines 989 through 992.

    if keywords is enabled
       and there is a scheme
       and the scheme does not start with http
    then keywords will not be used

So, it sounds like this could be fixed with a nice two-line patch, eh?

The comments between lin 980 and 988 indicate that the coder wants to clean that
part up at some point, but the existing safeguard is good enough for our purposes.

-matt

PS: modified summary to include additional likely search terms and reduce dups

Comment 10

[m_mozilla]

Samir, regarding your Comment #6

This bug happens when internet keywords is enabled.

The behavior is as if they were disabled, because the code disables internet
keywords if a dot character is found in the string. This makes it impossible to
use internet keywords to search for
    Mr. Ed
    Mr. T
    +site:foo.com +"download productnamehere"
etc.

I've got internet keywords set to google, and would love to use internet
keywords for all my searching, but for now, I can only use it for searches which
do not contain dot characters.

I think this is a very simple and low-risk fix.

-matt

Comment 11

[Samir Gehani]

Matt,
Hey!  Wanna take this one?

Comment 12

[m_mozilla]

uh...

well, I've never "owned" a bug before... but I'm not opposed to the idea. I
should note that I'm not a C programmer. My C experience is circa 1989 (that
year, and that year only) and I'm more of a perl guy. I'm fairly sure the fix is
reasonable, but I'm infering a great deal from surrounding context. PRInt32 is
some kind of 32bit integer I'm guessing. hostStr.FindChar is searching the whole
text typed in the URL field (minus initial/trailing whitespace) I'm guessing...
but I'm not certain.

Also, I don't have the mozilla source on my computer, nor am I well-clued with
respect to CVS (understand the general principle, but never used it myself).

If I knew for certain how to get the proper version of the file in question, I'd
easily do the diff -u, but I'm not even sure of how to get the proper version of
the file in question.

So, basically, I'm saying that I could take it, but I'd need a whole lot of
hand-holding for this two line patch. Is it worth your time to do the hand
holding or would you rather just do the patch? :/

-matt

Comment 13

[Joonas]

Sorry to bug you, but how is this bug doing? Is anyone interested in making the simple patch matt 
mentions in comment #9?

Comment 14

[benc]

This flawed "dot parsing" as a way of recognizing DNS hostnames came from some
the original psuedo-spec for IK in Navigator 4. [e-mail]'s (#6)
examples suggest that the Mozilla implementation is very flawed.

If you want this tweek fixed, the best way to increase the priority is to test
the examples in Navigator 4. If they work in Navigator 4, then mark this w/ the
keyword 4xp.

The real solution is the implement bug 127872, which would wipe out all
syntactical vagaries.

Comment 15

[m_mozilla]

The examples *do* work in NN4.

Specifically, in v4.78 (didn't have 4.79 on this machine), if I enable IK, and
type in Mr. T. I get 

    http://search.netscape.com/search.psp?cp=clkussrp&charset=UTF-8&search=mr.+t

with such wonderfull links as

    Mr. T. verus The Leprechaun
    http://willmistretta.tripod.com/versus.html

In mozilla build 2002022508 I get

    www.Mr. T. could not be found

I've not added a keyword before. I've put ", 4xp" on the end of the current
keywords string. We'll see if I have access to do that, and if that's in fact
how one adds a keyword...

-matt

PS: any chance this could make 0.9.9 ?

Comment 16

[m_mozilla]

Attachment: untested patch. Someone please test.

I'm a perl guy. Haven't done C stuff since high school. I also don't have a
build environment to test this.

Those caveats aside, I've found where the code moved with bonsai, I've gotten
the source via lxr (not cvs) and then made what seemed to be the appropriate
edits and done a diff.

Can someone with a real build settup test this? I understand the freeze for 1.0
is less than a week out. I'd really like to see this 4.x parity issue fixed.

Many thanks :)

-matt

Comment 17

[m_mozilla]

Attachment: trivial tweak to comments in above patch... still untested

modified quote style for additional phrases listed in comments to be consistent
with previously existing examples.

change only affects comments, not code

Comment 18

[Claudius Gayle]

submitting for nsbeta1
ADT, drivers, whoever's looking, can we please consider getting this in?
There is a simple patch attached. The benefits would be 4.x parity and a more
consistent behavior(the search button was already fixed to do this correctly).
Without this fix we're left with ugly and illogical results. Comment #15 is the
most straightforward example.
The URL Bar is of course the single most high-profile piece of UI and Keywords
are on by default, let's get this right.

Comment 19

[Peter Trudelle]

nsbeta1+ per Nav triage team, ->1.0, nav3

Comment 20

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 21

[Peter Trudelle]

converting nav3->adt3

Comment 22

[Peter Trudelle]

Mass moving nsbeta1+/adt3 bugs assigned to Navigator team engineers out to
target milestone 1.0.1.  Please confine your attentions to driving down our list
of TM 1.0 bugs for beta.  Better to help, debug, or test one of them than fix
one of these.

Comment 23

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 24

[scottputterman]

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt.  If you have any
questions about this, please email [e-mail].  You can search for
"changing adt3 bugs" to quickly find and delete these bug mails.

Comment 25

[John Levon]

*** Bug 135601 has been marked as a duplicate of this bug. ***

Comment 26

[John Levon]

*** Bug 123968 has been marked as a duplicate of this bug. ***

Comment 27

[u60234]

*** Bug 112672 has been marked as a duplicate of this bug. ***

Comment 28

[benc]

asa asked me to look into some aspects of this problem, so here is my update:

As other people have noted, "." causes IK to be bypassed, even if other
characters that should logically force a IK lookup (most notably spaces) are
present.

There are actually two problems here, and I'll illustrate by example:

"what is 3.14159" - should go to internet keywords, but instead is errored as
"an invalid protocol". Anything with spaces should go to IK if it is on (and
give this protocol error if this is off). In comparison, "what is 3" goes to
Internet Keywords.

"3.14159" - should go to DNS. If DNS fails with "hostname not found", then it
should be sent to Internet Keywords. In comparison, a single word, presumed to
be a hostname, is sent to DNS and then IK. In comparison, "3" will fail, and
then be sent to the keyword server.

In other words, the general expectation is that Internet Keywords should be used
 in two cases:
1- Whenever it is obvious (spaces is a giveaway), that the user typed something
that cannot be resolved in DNS.
2- Whenever DNS was searched and "hostname not found" was returned.

Right now, IK is sometimes used when it should not, and not used when it should.

Comment 29

[Vedran Miletic]

retargeting

Comment 30

[Asa Dotzler [:asa]]

Vedran Miletic, the target milestone field belongs to the bug assignee. Please
stop changing fields in Bugzilla. Thank you.

Comment 31

[Asa Dotzler [:asa]]

*** Bug 218918 has been marked as a duplicate of this bug. ***

Comment 32

[Asa Dotzler [:asa]]

*** Bug 236136 has been marked as a duplicate of this bug. ***

Comment 33

[Fredrik Öhrn]

(In reply to comment #32)
> *** Bug 236136 has been marked as a duplicate of this bug. ***

Bug 236136 notes that : (colon) doesn't work in keyword search, maby the summary
of this bug can be updated to include both . and :

/Fredrik

Comment 34

[:Gavin Sharp [email: [e-mail]]]

*** Bug 243786 has been marked as a duplicate of this bug. ***

Comment 35

[Jeff Walden [:Waldo]]

A couple months ago, I filed bug 261608, which is pretty much a dup of this one.
 However, I also did the work to create a patch, and I posted the patch pretty
quickly.  After a few iterations of review I finally got r/sr, and it was
checked in a few days ago.  With the patch checked in, this bug should now be
fixed.  If after testing some trunk nightlies people here agree this is so,
please mark this bug as a duplicate of bug 261608.  Thanks!

Comment 36

[:Gavin Sharp [email: [e-mail]]]

*** This bug has been marked as a duplicate of 261608 ***

Comment 37

[Elliotte Rusty Harold]

I don't appear to have an option to reopen this bug but it's not fully fixed
yet. In the latest Firefox beta a search for "Java 1.5" works. However a search
for "javax.usb" fails with a "Server not found". Similarly a search for
"java:test is a sample" fails with "Firefox doesn't know how to open this
address, because the protocol (java) isn't associated with any program." On the
other hand "sample java:test is a sample" succeeds. 

I think that is it isn't obviously a bad URL, then Firefox should default to a
search for the term (at least if keyword seacrh is enabled)