124674 - Disabling URL bar autocomplete does not work on Linux

Status: RESOLVED FIXED

(SeaMonkey :: Location Bar, defect)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD:  Linux 2002-02-07-07

STEPS TO REPRODUCE:

1)  Go to Edit > Preferences > Navigator > Smart Browsing
2)  Make sure location bar autocomplete is _on_.
3)  Click the "Advanced" button
4)  Uncheck "Autocomplete best match as you type"
5)  Check "Show list of matching results"
6)  Check "Show internet search engine"
7)  Click OK
8)  Turn _off_ location bar autocomplete in the "Smart Browsing" panel (note
    advanced button is disabled)
9)  Click OK on preferences dialog.
10) Start typing in URL bar

EXPECTED RESULTS:  you type and nothing happens

ACTUAL RESULTS:  a dropdown appears offering to search for the text you typed.
                 This dropdown grabs mouse focus.  There is no clear way to turn
                 this dropdown off in prefs.

NOTES:

If in step #6 you _uncheck_ "Show internet search engine" you still get a
dropdown.  It's empty, about two pixels tall, nearly invisible (unless you look
for it) and _still_ grabs mouse focus

The only way to not have a dropdown is to uncheck both "Show internet search
engine" and "Show list of matching results".  The overall "autocomplete
disabled" pref will affect whether results appear in the dropdown but not
whether the dropdown appears.

Comment 1

[Andrew Schultz]

see bug 97344 for the Internet search part.

Comment 2

[Andrew Schultz]

Attachment: patch

this prevents the popup from showing if there are no autocomplete results and Internet search is disabled.

Comment 3

[jag (Peter Annema)]

Comment on attachment 204859 [details] [diff] [review]
patch

       <method name="updatePref">

(NB: not directed at you)
Ugh... Those should really test against 0 instead of that hack-ish ! test, though I wonder if this expensive version of |StartsWith| is really the test you want to be doing in the first place.

Looks good, bonus points if you could fix up that pattern throughout this file. /^foo/.test(aPref) should work for the |StartsWith| test, and regular old string matching for when you don't want surprises.

Btw, have you noticed that when you turn off "Automatically complete text typed into the Location bar" and save, next time you go to that pref panel the "Advanced..." button isn't disabled?

Comment 4

[[e-mail]]

I'm not convinced this is correct. I'd WONTFIX this in favour of bug 97344.

(In reply to comment #3)
>Ugh... Those should really test against 0 instead of that hack-ish ! test,
>though I wonder if this expensive version of |StartsWith| is really the test
>you want to be doing in the first place.
Nah, they should be just doing ==

>Btw, have you noticed that when you turn off "Automatically complete text typed
>into the Location bar" and save, next time you go to that pref panel the
>"Advanced..." button isn't disabled?
Looks like that panel's missing a function Startup()

Comment 5

[Andrew Schultz]

> I'm not convinced this is correct. I'd WONTFIX this in favour of bug 97344.

hmm?  Why shouldn't a user be able to disable autocomplete and still use the search functionality (I thought that's also what your comment in bug 97344 meant)?  
It's hard to set that with the current pref UI, but it seems like a reasonable thing for a user to want.

The patch also fixes the bug that if you have search disabled but autocomplete on but no results the popup still opens (0px tall) and prevents you from switching focus to another window.

Comment 6

[jag (Peter Annema)]

Regardless of the rest, we should fix the bug that when autocomplete is disabled, "matching results" is checked and "search engine" is unchecked we get this 0px tall popup.

If I understand Neil correctly, I agree with him. If autocomplete is disabled, there shouldn't be a popup.

I like the idea of not showing the history matches but still being able to show the search engine item, though. Not that hard to code up, but it would require updating the "Advanced..." UI to disconnect the "search engine" option from the "matching results" option, and provide updated images for the new combinations.

Of course, if all you've got turned on is the "search engine", it's not really an autocomplete thing anymore. Maybe we can change the wording a bit? Or, with more work, split the search engine thing out, it could go into the "Smart Browsing" panel (minus the nice feedback images).

Btw, shouldn't "Match only websites you've typed previously" be visually tied to "autocomplete as you type" and/or "matching results" being selected?

Btw2, is this prefs overkill? How many people really want to turn off the search engine item, or the matching results? I can understand the "autocomplete as you type" one, it drives me nuts when it's on, but I can see others loving it. Not so for those other two though.

Comment 7

[[e-mail]]

(In reply to comment #6)
>If I understand Neil correctly, I agree with him.
No, you didn't, but I've changed my mind and I now agree with you :-)

Comment 8

[[e-mail]]

(In reply to comment #5)
>I thought that's also what your comment in bug 97344 meant
Hey, I don't even remember commenting in bug 97344 :-P

Comment 9

[[e-mail]]

Comment on attachment 204859 [details] [diff] [review]
patch

>           if (!aPref.indexOf("browser.urlbar.showPopup")) {
>             this.showPopup = this.mPrefs.getBoolPref("browser.urlbar.showPopup");
>           } else if (!aPref.indexOf("browser.urlbar.autoFill")) {
>             this.autoFill = this.mPrefs.getBoolPref("browser.urlbar.autoFill");
>-          } 
>+          } else if (!aPref.indexOf("browser.urlbar.showSearch")) {
>+            this.minResultsForPopup = this.mPrefs.getBoolPref("browser.urlbar.showSearch") ?
>+                                      0 : 1;
>+          }
You could use aPref == "browser.urlbar.showSearch" (or convert to a switch) which would then make it more obvious to use getBoolPref(aPref) which would make the line shorter ;-)

Comment 10

[jag (Peter Annema)]

Of course, the patch as is doesn't implement all of what I suggested in my previous comment. In fact, I believe right now, even with autocomplete turned off, you'd still always get the search engine entry.

/me wonders if he should change his + to a -.

Comment 11

[Andrew Schultz]

(In reply to comment #10)
> Of course, the patch as is doesn't implement all of what I suggested in my
> previous comment. In fact, I believe right now, even with autocomplete turned
> off, you'd still always get the search engine entry.

The patch implements the behavior -- "I like the idea of not showing the history matches but still being able to show the search engine item, though."  It's the prefs UI that remains broken (that's bug 97344).  The Internet search pref ought to be disconnected from autocomplete.  And yes, with autocomplete off, you still get the search engine entry (if you have the Internet search pref enabled).

Comment 12

[Andrew Schultz]

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

Comment 13

[Andrew Schultz]

Comment on attachment 204859 [details] [diff] [review]
patch

This patch is in with jag's suggestions from comment 3 (regexp and == instead of indexOf)

I noticed in a final test that the URL bar resists this bug being fixed because it explicitly specifies alwaysopenpopup=true
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator.xul#184

I'll leave this bug open in hopes of removing that.

Comment 14

[[e-mail]]

Maybe it needs to work off the search field setting too?

Comment 15

[Andrew Schultz]

The inconsistent use of the setting is what really bothers me.  Why does it use different criteria for opening/closing the popup.

URL bar is the only thing we have that uses alwaysOpenPopup and it would work better if the setting didn't exist.  Why not just do everything based on minResultsForPopup?

Comment 16

[Andrew Schultz]

Attachment: nuke alwaysopenpopup

Comment 17

[Andrew Schultz]

Attachment: the right patch

<sigh>

Comment 18

[Andrew Schultz]

fixed for real this time