332668 - keyword.enabled probably doesn't work any more

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[[e-mail]]

roc changed the keyword.enabled backend in bug 331522 but we need xpfe changes.

Comment 1

[Frank Wein [:mcsmurf]]

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

Comment 2

[jag (Peter Annema)]

*grmbl*

So, shall I just add that flag to all loadURIWithFlags calls, or do we too wanna disable keyword loads for certain cases?

Comment 3

[[e-mail]]

It would be nice if we had a list of all callers, then we could see how easy it would be to selectively apply the flag.

Comment 4

[[e-mail]]

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

Comment 5

[Curtis Williams]

Does anyone know if there is a workaround to this?  This is something I *used* to do all of the time.  It's hard to believe more people aren't complaining about this being gone.

Comment 6

[Robert Kaiser]

What does this bug mean?
Without seeing what really is broken or not it's hard to decide if we should hold a release (or even an Alpha) for this...

Comment 7

[Curtis Williams]

This means that if you have  "Enable Internet Keywords"  checked in the preferences that it doesn't work anymore.  In other words, if I wanted to search for something, I would simply type it into the location bar and hit return and it would automatically do the search based on what I have set for Internet Search in the preferences.

Comment 8

[Robert Kaiser]

Keywords and Internet Search are two different beasts, from what I know, so I'm not sure if you are right.
And if I set a keyword "bug" for %s-based Bugzilla lookup, it works well in a branch build...

Comment 9

[Andy Willis (abwillis)]

I believe this is keyword.enabled set to true in about:config is what allowed me to search from the url bar with an enter click.  It was a much smarter search system than IE has... IE decides to search for every blasted thing that it can't find which sucks when you just mistype a url.  Seamonkey would only search with an enter if there was a space in the url bar (indicating it was not a url) or if the format was keyword:search  where search was the word being searched for.  As I rarely do a one word search it worked beautifully.  I had thought I had just messed up my prefs.js as I had had to specify adding that ability expressly and didn't realize it had been regressed intentionally.  I for one would really like to see this as a blocker I am constantly getting an error message that the input was not a valid URL (really even getting rid of the error would be better so I didn't have to acknowledge it before hitting the search button would be OK).

Comment 10

[Robert Kaiser]

So, if I understand you correctly and this is just about the issue that calling up the default search engine by pressing enter in the URL bar for any string we can't handle with other methods, this is something that would really be great to have (I'd love it as well), but we won't block a release for it.

Comment 11

[Curtis Williams]

That's up to you guys, but from a general user perspective it's a previously released feature with a configuration option that now doesn't work - a regression.

Comment 12

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Re-request blocking for the beta.

Comment 13

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Reading the Firefox patch, I suspect we could make the last (new) argument optional, default to false, and that would make the patch a lot smaller.

Comment 14

[Robert Kaiser]

I'm still not sure what this really is about and how important this is, so putting it on 1.1b radar.

Comment 15

[Adam Hauner]

KaiRo: Firefox has seperated searchbox. SeaMonkey has Location Bar with search capability. This search is now broken. (I'm using 1.0.x releases instead 1.1 nightly because this bug.)

Comment 16

[Andy Willis (abwillis)]

Just to keep things from being confused...  The search does work if you click the search button.  Previously you could type keyword:searchterm  and hit enter (or if you were entering 2 or more terms separated by spaces you could drop the keyword: part... not sure what if anything else caused the keyword to be assumed but it wasn't brain dead like i.e. and try to search for mistyped urls).  The search does still work but now it requires using the Search button rather than being able to use the enter key.  Here is some information on it  http://kb.mozillazine.org/Using_keyword_searches

Comment 17

[Robert Kaiser]

That's what confuses me, because I can't remember having that working in 1.0.x - I thought this has been broken since before the SeaMoneky project had been founded.

Comment 18

[Andy Willis (abwillis)]

To be honest, I am not sure when it broke... I build from Trunk and don't know the corresponding release information.  I had thought I had just borked my prefs.js and thought I was setting it back up wrong at first (the part I thought  I was setting up wrong I didn't at first realize wouldn't affect the issue I was seeing... I just remembered I had to make a couple of changes to get it to work as I wanted it to originally).  

Comment 19

[[e-mail]]

Attachment: fix urlbar

open location dialog sucks, will fix later

Comment 20

[Andrew Schultz]

Comment on attachment 234669 [details] [diff] [review]
fix urlbar

Patch looks good.  I hit bug 349419 with the patch applied, but so does firefox, so I don't think it's our misuse of the API.

Comment 21

[[e-mail]]

Attachment: fix open location dialog

requires attachment 234669 [details] [diff] [review]

Comment 22

[Karsten Düsterloh]

While we probably may not want to halt an alpha for this, breaking the "enter search terms in urlbar and hit enter" feature is definitely a 1.1 blocker, imo.

Comment 23

[Andrew Schultz]

Comment on attachment 234700 [details] [diff] [review]
fix open location dialog

>@@ -63,7 +63,7 @@ function editDocument(aDocument)      

as long as you're cleaning things up :) editDocument is never called.

>-function editPage(url, launchWindow, delay)
>+function editPage(url)

xpfe's (and toolkit's!) viewsource has a ViewSourceEditPage method that still passes 3 parameters.

OpenURLDialog in xpfe/components/xremote/src/XRemoteService.cpp also needs updating

Are you yanking the delayed open because it's icky or something directly related to this bug?

Comment 24

[[e-mail]]

(In reply to comment #23)
>>-function editPage(url, launchWindow, delay)
>>+function editPage(url)
>xpfe's (and toolkit's!) viewsource has a ViewSourceEditPage method that still
>passes 3 parameters.
Fixed locally.

>OpenURLDialog in xpfe/components/xremote/src/XRemoteService.cpp also needs
>updating
That sucks. How much do you want to keep OpenURLDialog? It got overlooked when I implemented openurl for suiterunner.

>Are you yanking the delayed open because it's icky or something directly
>related to this bug?
Because it's icky, and I don't really want to have to touch all the functions that it uses to make them keyword-enabled.

Comment 25

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 234669 [details] [diff] [review]
fix urlbar

Diffing from xpfe/ has the wonderful result of making the Index: line look like a Firefox patch.

Comment 26

[[e-mail]]

Attachment: addressed review comments

Comment 27

[Andrew Schultz]

Comment on attachment 234992 [details] [diff] [review]
addressed review comments

patch: **** malformed patch at line 146: Index: suite/common/openLocation.js

@@ -146,17 +129,12 @@ function editPage(url, launchWindow, del
should be
@@ -146,12 +129,7 @@ function editPage(url, launchWindow, del
?

>+++ xpfe/browser/resources/content/navigator.js	20 Aug 2006 20:35:33 -0000
>+  openDialog("chrome://communicator/content/openLocation.xul", "_blank", "chrome,modal,titlebar", params);
>+  var url = getShortcutOrURI(params.url);

getShortcutOrURI doesn't like null, so this needs to bail early if the user cancels the dialog.

Comment 28

[[e-mail]]

(In reply to comment #27)
>(From update of attachment 234992 [details] [diff] [review])
>patch: **** malformed patch at line 146: Index: suite/common/openLocation.js
>@@ -146,17 +129,12 @@ function editPage(url, launchWindow, del
>should be
>@@ -146,12 +129,7 @@ function editPage(url, launchWindow, del
>?
You should have seen the patch before I cut out all my other junk...

>>+++ xpfe/browser/resources/content/navigator.js	20 Aug 2006 20:35:33 -0000
>>+  openDialog("chrome://communicator/content/openLocation.xul", "_blank", "chrome,modal,titlebar", params);
>>+  var url = getShortcutOrURI(params.url);
>getShortcutOrURI doesn't like null, so this needs to bail early if the user
>cancels the dialog.
I'll make it default to "" instead.

Comment 29

[[e-mail]]

Attachment: fixed patch

Comment 30

[[e-mail]]

(In reply to comment #27)
>@@ -146,17 +129,12 @@ function editPage(url, launchWindow, del
>should be
>@@ -146,12 +129,7 @@ function editPage(url, launchWindow, del
Doh, I corrected the 12 to 7 but forgot to correct the 17 to 12 :-[

Comment 31

[[e-mail]]

Fix checked in.

If anyone wants internet keywords to work anywhere else, please explain your reasoning before reopening this bug, thanks.

Comment 32

[[e-mail]]

Fixed on 1.8 branch 2 days ago but I forgot to update the keywords.

Comment 33

[Wladimir Palant]

Removing delayedOpenTab will probably break lots of extensions. Actually, there is even one occasion where SeaMonkey itself still tries to use this function - gTabOpenObserver.observe() in navigator.js. This should be fixed.

Comment 34

[benc]

V/fixed.
Seamonkey 1.1.2, macosx10.4.