130462 - "null" or "" is displayed in drop-down search text in URL bar

Status: RESOLVED FIXED

(SeaMonkey :: Location Bar, defect)

Description

[Rui Xu]

Steps:
1. Launch browser.
2. Put cursor into URL bar.
3. Press Up Arrow.

Please refer to the attachment.

Comment 1

[Rui Xu]

Attachment: In comm. trunk build, "null" is displayed in drop-down search text

Comment 2

[Boris Zbarsky [:bzbarsky]]

Rui, which build are you using? There was an XBL regression earlier today that
could have caused this....

Comment 3

[Boris Zbarsky [:bzbarsky]]

Oh, and if this is commercial-only, please move this to bugscape..

Comment 4

[Rui Xu]

Attachment: In Mozilla build, " " is displayed in drop-down search text

Comment 5

[Rui Xu]

Can repro this bug in both Mozilla and comm. trunk build.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Rui, which exact trunk build? Like "at what exact time yesterday did you check
out?"  See comment 2.  There was a window of a few hours when most of XBL was
broken (and autocomplete is done in XBL).

Comment 7

[Rui Xu]

I spot-checked several builds on Windows:

1. "null" is displayed in the followings:
  1.1. Comm. trunk:   20020313, 20020306
  1.2. Mozilla trunk: 20020304

2. "" is displayed in the followings:
  2.1. Comm. trunk:   20020221, 20020204, 20020122, 20020114
  2.2. Mozilla trunk: 20020311, 20020204, 20020122, 20011221

Comment 8

[Boris Zbarsky [:bzbarsky]]

Heh.  Ok. :)

To URL bar to look at this, then.

Comment 9

[Itamar]

I'm gonna have a look at this one...

Comment 10

[Itamar]

after looking around, fixing this seems easy to do.
The main problem is what to fix it to.
I agree that "null" should not be there - but should it be the current string in
the url bar? a blank? a space?
currently if you search for a string using the search menu item the next time
you hit the down key that same search string will appear there.
If this is ok then my best guess is that an empty string "" should replace the
(null)
Any comments on this?

Comment 11

[Rui Xu]

Could we enable that sentence displayed only if the search menu item is not 
empty?

Comment 12

[Itamar]

I guess it could be done but I think it would be a bit confusing that somtimes
the search bar apears and somtimes not.
I selected the search bar several times just to get to google web site - like a
bookmark that is always there.

Comment 13

[Matthias Versen [:Matti]]

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

Comment 14

[Oded Rotem]

This can also occur if the location bar is NOT empty. (Build 2004042109)
1. Open a new browser window.
2. Enter "one two" in the location bar and press Enter.
3. The message "The URL is not valid [etc]" will appear.
4. However, the location "one two" will be added to the location history list.
5. Open a new browser window, and click the location bar arrow in order to open
the history pull down.
6. Select "one two".
7. The above error message will appear again.
8. However, this time, if you set focus on the location bar text and press the
down key, it will display "search for Null", instead of "search for "one two" ".

Comment 15

[Andrew Schultz]

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

Comment 16

[Andrew Schultz]

Attachment: initialize to an empty string

Comment 17

[[e-mail]]

Comment on attachment 202462 [details] [diff] [review]
initialize to an empty string

This doesn't address comment 14; I guess someone is incorrectly using currentSearchString which looks as if it should be internal to autocomplete.

Comment 18

[jag (Peter Annema)]

That patch fixes the bug in that it replaces "(null)" with "", but as Neil pointed out there are other problems here.

Playing with this, I'm rather surprised when I open a new tab, hit up or down, and find a bunch of search results even though I didn't type anything. I'd expect either nothing (as is the case for opening a new window), or everything in my location history.

Then there's a third issue, as pointed out in comment 14, that if you open a new window and load a page through e.g. a bookmark or by selecting a value from the urlbar dropdown, if you then hit down or up in the urlbar you'll also get an empty list instead of one that uses the urlbar's current value to match against. In a way it makes sense, since you didn't type it, but I'd rather we use the current value as if the user had typed it.

Comment 19

[Andrew Schultz]

> Playing with this, I'm rather surprised when I open a new tab, hit up or down,
> and find a bunch of search results even though I didn't type anything.

Do you mean a bunch of autocomplete results?  The (autocomplete and search) results are based on the last thing you typed in the URL bar for any tab.  Autocomplete is tab-agnostic.

> I'd rather we use the current value as if the user had typed it.

I had considered:

-            this.mSearchBox.searchValue = this.textbox.currentSearchString;
+            this.mSearchBox.searchValue = this.textbox.value;
(in urlbarBindings.xml)

It does what you suggest, but isn't usable because if you hit "down" again after triggering the dropdown, you select the first autocomplete URL, which changes the text in the URL, which changes the search text.

Comment 20

[jag (Peter Annema)]

(In reply to comment #19)
> > Playing with this, I'm rather surprised when I open a new tab, hit up or down,
> > and find a bunch of search results even though I didn't type anything.
> 
> Do you mean a bunch of autocomplete results?  The (autocomplete and search)
> results are based on the last thing you typed in the URL bar for any tab. 
> Autocomplete is tab-agnostic.

Yes, "a bunch of autocomplete results" is what I meant. I understand what it's based on, but I don't think it makes sense that when I hit the down arrow in a new tab I get the autocomplete results for what I typed in another tab, more so when the new tab opens to my homepage.

Related to this is when I go to e.g. google by typing it in the urlbar, then I go somewhere else by clicking a link or a bookmark, now when I hit up or down in the urlbar I get the autocomplete results for google, while the urlbar value is e.g. "http://slashdot.org/". In this case it's easy for me to figure out what's going on, but if it's been a few link/bookmark clicks and a few tabs ago that I actually typed into the urlbar, the mismatch between the urlbar value and the autocomplete results make no sense.

> > I'd rather we use the current value as if the user had typed it.
> 
> I had considered:
> 
> -            this.mSearchBox.searchValue = this.textbox.currentSearchString;
> +            this.mSearchBox.searchValue = this.textbox.value;
> (in urlbarBindings.xml)
> 
> It does what you suggest, but isn't usable because if you hit "down" again
> after triggering the dropdown, you select the first autocomplete URL, which
> changes the text in the URL, which changes the search text.
 
Can we update the currentSearchString from the BrowserStatusHandler when a page is loaded?

Comment 21

[Andrew Schultz]

> Can we update the currentSearchString from the BrowserStatusHandler when a page
> is loaded?

Yes.  That fixes the search string, but autocomplete doesn't pick it up.  Calling processInput() instead of just setting currentSearchString fixes both search and autocomplete, but only if the url bar has focus:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml&rev=1.119#1100

Also, calling processInput sometimes causes the dropdown to show up (see line 610)

Comment 22

[jag (Peter Annema)]

Sounds like we need to refactor that function then.

Comment 23

[[e-mail]]

(In reply to comment #19)
>I had considered:
>
>-            this.mSearchBox.searchValue = this.textbox.currentSearchString;
>+            this.mSearchBox.searchValue = this.textbox.value;
>(in urlbarBindings.xml)
> 
>It does what you suggest, but isn't usable because if you hit "down" again
>after triggering the dropdown, you select the first autocomplete URL, which
>changes the text in the URL, which changes the search text.
That's not a problem; regular autocomplete does that anyway. Once you down enough times to hit the search text, it reverts to your original search text.

Comment 24

[Andrew Schultz]

Attachment: process URL bar value when user hits DOWN

I had a working patch to invoke processInput from onLocationChange, but this is a lot simpler.

Comment 25

[[e-mail]]

Comment on attachment 204198 [details] [diff] [review]
process URL bar value when user hits DOWN

I like this.

Comment 26

[jag (Peter Annema)]

Comment on attachment 204198 [details] [diff] [review]
process URL bar value when user hits DOWN

Nice!

Comment 27

[Andrew Schultz]

checked in

Comment 28

[Andrew Schultz]

Comment on attachment 204198 [details] [diff] [review]
process URL bar value when user hits DOWN

Low-risk polish patch (baked for 1+ month).  Code shared with tbird.

Comment 29

[Andrew Schultz]

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