search bar autocomplete popup (history, suggestions) doesn't work

VERIFIED FIXED in Firefox 3 beta2

Status

()

Firefox
Search
P1
normal
VERIFIED FIXED
10 years ago
7 years ago

People

(Reporter: Paenglab, Assigned: Gavin)

Tracking

({regression})

Trunk
Firefox 3 beta2
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112405 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112405 Minefield/3.0b2pre

Since the landing of Bug 398020 the search suggestions don't appears

Reproducible: Always

Steps to Reproduce:
1. Choose Google as search engine
2. Search for example the word mozilla
Actual Results:  
No list with search examples appears

Expected Results:  
List with search examples appears

Comment 1

10 years ago
Works for me, using Google, Yahoo and Wikipedia
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112405 Minefield/3.0b2pre
(Reporter)

Comment 2

10 years ago
Strange, now it works for me again.
I tested it with a new profile and the hourlys. It never worked with the hourlys after the landing of Bug 398020.

I close this bug
Status: UNCONFIRMED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → INVALID
I can reproduce this on Windows XP. Curious to know what made it work for the reporter; maybe only in shiny new profiles? :).
Regression range is http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1195882020&maxdate=1195884479
Blocks: 398020
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 4

10 years ago
Ria try to change the search engine and then back again. I am not shure, but I think, I've done this before the check after the post from Jo.
Search suggestions work for me using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112405 Minefield/3.0b2pre .

Extension-related, perhaps? Any errors in Error Console (with javascript.options.showInConsole set)?.
WFM on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112404 Minefield/3.0b2pre ID:2007112404
No, new profile, first run. I tried changing search engines, but no effect. The previous build worked.
What could be going on.

Updated

10 years ago
Keywords: qawanted, regression
Version: unspecified → Trunk
(Reporter)

Comment 8

10 years ago
I think I found it: open FF, then try to search something -> no suggestions. Now type a letter into the url bar (no enter needed), go to the search bar and search again -> voila the suggestions appears.

Tryed with a new profile and my old one. It's the same.
Yes exactly.
(In reply to comment #8)
> I think I found it: open FF, then try to search something -> no suggestions.

Does this require your search history to be empty? If it isn't empty, do the history entries show up, but not suggestions?

Comment 11

10 years ago
Once the workaround in comment #8 is performed, I get both search history and suggestions.  Prior to workaround, neither.

(Reporter)

Comment 12

10 years ago
(In reply to comment #10)
> (In reply to comment #8)
> > I think I found it: open FF, then try to search something -> no suggestions.
> 
> Does this require your search history to be empty? If it isn't empty, do the
> history entries show up, but not suggestions?

It doesn't matter. As I wrote, no difference if a new or a old profile with search history. It appears no list (no suggestions and no history).

IMHO it has something to do with the new mechanism of the go-button. Only after the button appears one time all works as exepted.
Flags: blocking-firefox3?

Updated

10 years ago
Assignee: nobody → dao

Updated

10 years ago
Blocks: 402668
Keywords: qawanted
The same is happening for the content area due to bug 402668.
Assignee: dao → nobody
Why did the landing of bug 398020 trigger this? Or is that just wrong?
Because the search bar isn't using the openAutocompletePopup method from urlbar-result-popup.

But I'm not sure anymore that this caused a problem for the content area -- I can't reproduce this right now.
marking as all, as I see this on the mac as well.

note, as dao points out, in search.xml we have an openPopup method now:

http://lxr.mozilla.org/seamonkey/source/browser/components/search/content/search.xml#650

should the fix be to add something this this to openPopup() in search.xml?

+          // initially the panel is hidden
+          // to avoid impacting startup / new window performance
+          popup.hidden = false;
OS: Windows XP → All
Hardware: PC → All
that seems to fix it for me, I'll attach a patch and seek review from dao / gavin.
Assignee: nobody → sspitzer
Created attachment 290062 [details] [diff] [review]
patch
Attachment #290062 - Flags: review?(dao)
Comment on attachment 290062 [details] [diff] [review]
patch

This works, but I'd only do it if the popup isn't open.
Attachment #290062 - Flags: review?(dao) → review+
Created attachment 290063 [details] [diff] [review]
updated patch, per dao

note to dao, I have not been able to reproduce this problem with the content area.
Attachment #290062 - Attachment is obsolete: true
Attachment #290063 - Flags: review?(dao)

Updated

10 years ago
Attachment #290063 - Flags: review?(dao) → review+
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 3 M10
Attachment #290063 - Flags: approval1.9?
When I wrote comment 13, autocomplete in the content area started working once I had typed in the Location bar. But it seems that this was random. I know I've seen that problem several times before, but it comes and goes unpredictably. Anyway, it's most likely not a regression from bug 402668.
Target Milestone: Firefox 3 M10 → ---
Flags: in-litmus?
Target Milestone: --- → Firefox 3 M10
Comment on attachment 290063 [details] [diff] [review]
updated patch, per dao

If the search bar isn't using the urlbar-result-poppup anymore, that's the bug we should fix.

The check-in for bug 398020 does appear to regress bug 295498 for the search bar.
Attachment #290063 - Flags: review-
Comment on attachment 290063 [details] [diff] [review]
updated patch, per dao

obsoleting, based on gavin's comment.
Attachment #290063 - Attachment is obsolete: true
Attachment #290063 - Flags: approval1.9?
Created attachment 290068 [details] [diff] [review]
remove the openPopup in search.xml
Attachment #290068 - Flags: review?(gavin.sharp)
Comment on attachment 290068 [details] [diff] [review]
remove the openPopup in search.xml

This regresses the alignment of the popup (its left edge doesn't match up with the left edge of the typable area). Per discussion with dao on IRC I think we want to factor out the parts of _openAutocompletePopup in autocomplete.xml that get the information used to calculate width and the context element to pass to openPopup and have the search bar binding override that explcitly.
Attachment #290068 - Flags: review?(gavin.sharp) → review-
Created attachment 290077 [details] [diff] [review]
alternate approach

This also fixes bug 405269.

<gavin> dao: http://pastebin.mozilla.org/248842
<dao> gavin, this.mInput.className == "searchbar-textbox" looks fragile, too
<gavin> well, I guess so
<gavin> could use the code from your patch
<gavin> that uses browsersearch
<gavin> the other thing is that urlbar-result-popup now overrides both _setWidthAndOpenPopup and openAutocompletePopup
<gavin> which is kind of messy
<dao> I kinda like it
Attachment #290077 - Flags: review?(dao)
Attachment #290077 - Flags: review?(sspitzer)
(In reply to comment #26)
> <gavin> could use the code from your patch
> <gavin> that uses browsersearch

(attachment 290071 [details] [diff] [review])
I also want to improve the comments before landing this, because I did this quick. I just wanted to get it up here and see what you guys think of the approach in general.
(In reply to comment #26)
> <gavin> the other thing is that urlbar-result-popup now overrides both
> _setWidthAndOpenPopup and openAutocompletePopup
> <gavin> which is kind of messy
> <dao> I kinda like it

what I like is the _setWidthAndOpenPopup idea, not that urlbar-result-popup is becoming messy :)
1)

I see what dao was pointing out (about having search.xml depend on
urlbarBindings.xml)

but I guess we already have code that does that in urlbarBindings.xml (in the
onPopupClick method)

is this necessary because the searchbar binding, unlike the urlbar binding,
doesn't extend the autocomplete binding?

2)

Good point / comment about:  

// Fall through for now, bug 397594 might change this

if this lands, we should go put a comment in that bug referring back to this
code.

Over to gavin.
Assignee: sspitzer → gavin.sharp
Status: ASSIGNED → NEW
Comment on attachment 290077 [details] [diff] [review]
alternate approach

Using BrowserSearch.searchBar for the comparison and doSearch still seems better to me. What you have now would support multiple search bars, but I don't think we need that (as with the urlbar).

The final patch should probably remove openPopup from search.xml.

And yes, it's confusing that this binding is called "urlbar-result-popup" and lives is in urlbarBindings.xml.
Attachment #290077 - Flags: review?(dao)
btw, I think you don't need to override openAutocompletePopup anymore ... |this.hidden = false| in _setWidthAndOpenPopup should do the trick.
Comment on attachment 290077 [details] [diff] [review]
alternate approach

clearing review request, sounds like gavin / dao have a plan of attack.
Attachment #290077 - Flags: review?(sspitzer)
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P1
Status: NEW → ASSIGNED
Summary: Regression: No search suggestions since landing of Bug 398020 → search bar autocomplete popup (history, suggestions) doesn't work until the URL bar autocomplete popup has been shown

Updated

10 years ago
Duplicate of this bug: 406303

Comment 35

10 years ago
I filed Bug 406303, having never experienced this issue before the 20071130 build on WinXP. Before filing it, I went back and installed 20071129, no problem whatsoever, no need to do any of the steps listed in this(405235) Bug to reproduce the condition. Then just now saw my Bug had been marked as a Duplicate of this, so tried all of the suggested Steps To Reproduce, and nothing works. Therefore I believe 406303 to be a separate issue, and valid.
(In reply to comment #35)
> Therefore I believe 406303 to be a separate issue, and valid.

If you make the location bar dropdown appear (by typing something into the location bar, or clicking the dropdown button), can you reproduce bug 406303?

Comment 37

10 years ago
(In reply to comment #36)
> If you make the location bar dropdown appear (by typing something into the
> location bar, or clicking the dropdown button), can you reproduce bug 406303?

Yes. Sorry if I was unclear. To restate what I observe: starting with 20071130, no matter what I do, which includes typing in the Location Bar (observing the Star change to the 'Go' arrow), submitting what I've typed in the LB either by pressing Enter or by clicking the 'Go' button, clicking the LB dropdown, clicking the LB dropdown and selecting and visiting a displayed item, etc -- Google Suggest does not work, and no errors are present in the Error Console. Hence, why I filed Bug 406303 as a separate Bug.
Ah, what about content autocomplete? If you get the dropdown to show up for a text input or password field, I bet it will work then. The patch for bug 399664 made us use a different popup for the location bar, so that hackaround doesn't work anymore.
Summary: search bar autocomplete popup (history, suggestions) doesn't work until the URL bar autocomplete popup has been shown → search bar autocomplete popup (history, suggestions) doesn't work

Comment 39

10 years ago
Ah very good.(In reply to comment #38)
> Ah, what about content autocomplete?

Ah, super good call! I think you just cracked this one + as a bonus, made a interesting discovery concerning Bug 404135 ("Restore Default Set.. breaks the location bar and the search bar"). The reason I'd come across both is because I was using new profiles to test and therefore had no search/content-autocomplete history; when there is an autocomplete history available, the Bug is not present!

Steps to Reproduce what I was seeing in Bug 406303:
1. Create a brand new profile. Start Minefield.
2. In the *toolbar* Search Box with the default "Google" engine already selected, start typing a word, ex: "firefox" but do *not* hit Enter or click the Search Submit button; expected behaviour: as you type the Google Suggest pop-up should appear with list of suggestions; actual behaviour: nothing appears.
4. backspace/delete all of the typed characters
5. The 'firstrun' page that loads by default - http://www.mozilla.org/projects/firefox/3.0b2pre/firstrun/ - will do for this next step - we need a page with a content-area text box, such as a searchbox.
- Enter a word, such as "firefox" into the *in-page* text-box - and press Enter/Submit button.
6. Just to test things, click back in the toolbar Search Bar and start typing any word - eg: "firefox", but do not hit Enter. Still no Google Suggest appears. Delete all of the characters again.
7. Click the 'Back' button to return to the original page. The original word "firefox" will be present in the in-page text box.
8. Double-click the text box(/"firefox" text). The autocomplete dropdown appears below the textbox with the word "firefox" in it. We have now activated autocomplete!
9. Now focus the toolbar Search Bar, and start typing "firefox" again. BINGO! After a character or 2, Google Suggest kicks in!!


Now the really neat part (that ties in with Bug 404135) is that if you then:
10. Right click on a toolbar and choose 'Customize' to open the Customize Toolbar palette.
11. Click 'Restore Default Set', then 'Done' to exit the pane
12. Repeat Step#2 from above.... you'll notice Google Suggest is now broken
13. Then click in the Location Bar.... you'll notice it's now acting weird. Even the dropdown arrow no longer works. Highlight all the text in the LB and delete it.
14. Then type "firefox.com" and press Enter. Nothing happens!
15. Repeat steps #7 and #8; we have now re-activated autocomplete!
15. Repeat Step #14. It works! As does the Search Bar!

I'll post a note in 404135 pointing to this Bug. If I'm up the (wrong) garden path, let me know.. ;0)
(Reporter)

Comment 40

10 years ago
I can confirm this. With browser.urlbar.richResults=true, I have to use content autocomplete for working search bar autocomplete.
With browser.urlbar.richResults=false, using the url bar autocomplete is enough.
Created attachment 291250 [details] [diff] [review]
yet another approach (diff -w)

Boris pointed out on IRC that bindings are attached whenever the node is accessed, to permit calling methods on the element even when it is display:none. This means we can use the binding constructor to unhide the panel and avoid having to do it from the openAutocompletePopup method, which in turn means that other users that override that method (or methods that call it), like the search bar, don't need to explicitly unhide the popup themselves.

The only question is whether this means we'll unhide the panel in the startup path, and thus lose the startup win from bug 402668. This could happen if the binding for the panel is applied on startup. That would only happen if someone tries to access the panel during startup, though, and I don't think that's the case. I'll have to watch closely at the perf numbers if I land this. If it does cause a perf regression we can go with one of Seth's earlier two patches for B2 while we figure out something better.
Attachment #290068 - Attachment is obsolete: true
Attachment #290077 - Attachment is obsolete: true
Attachment #291250 - Flags: review?(sspitzer)
I ran this latest patch through Txul locally, about 5 times with and 5 times without the patch, and was not able to identify a perf regression caused by the patch.
Comment on attachment 291250 [details] [diff] [review]
yet another approach (diff -w)

r=sspitzer, thanks for double checking Txul locally.
Attachment #291250 - Flags: review?(sspitzer) → review+
in bug #402668, I had the following note:

> 3)  extend this fix for the other popups / panels

following up:

1)  I think we have five panels, and you've fixed 3.

could we get some more txul / ts wins by doing this to:

<panel id="customizeToolbarSheetPopup" noautohide="true">

we also have, but I don't know if we are doing my hack or your better way, in
the ctor.

<panel id="identity-popup" position="after_start" hidden="true"
noautofocus="true">

2) what about for the other popups / tooltips in browser, in the mainPopupSet?

Thoughts?  I can make these spin off bugs.
Yeah, I think we should file some spinoff bugs on those... and probably one on making the hack unnecessary by improving panel initialization performance, if it isn't already filed.
Keywords: checkin-needed
Whiteboard: [needs landing]
I tried landing this earlier today:
mozilla/toolkit/content/widgets/autocomplete.xml 	1.95
mozilla/browser/base/content/urlbarBindings.xml 	1.46

but had to back it out because it regressed perf. The binding is probably attached early because we need it to call attachToBrowser, which happens during startup. I suppose we should just land attachment 290063 [details] [diff] [review] for b2.
gavin, fwiw, I agree that we should land https://bugzilla.mozilla.org/attachment.cgi?id=290063 (a less-than-perfect-fix) for this bug.

we could log a spin off bug about the other way (which is cleaner, but regresses Ts / Txul.)

as for comment #44, I think this makes my spin off bugs moot, except we can still reduce Ts / Txul by applying the "hidden trick" to <panel id="customizeToolbarSheetPopup" noautohide="true">.

I've logged bug #406860 on this issue.
Created attachment 291531 [details] [diff] [review]
what I'll land

This is attachment 290063 [details] [diff] [review] with an adjusted comment.
Attachment #291250 - Attachment is obsolete: true
Attachment #291531 - Flags: review+
Landed that patch for b2:
mozilla/browser/components/search/content/search.xml 	1.113

I'm kind of undecided about whether I really want to pursue landing the previous patch (attachment 291250 [details] [diff] [review]). This patch that did land (attachment 290063 [details] [diff] [review]) isn't really as messy as some of the alternatives, so I'm less concerned about cleaning this up than I was before, after having thought about this a bit. I think ideally we'd probably want to take something more like attachment 290077 [details] [diff] [review], but that's something for a followup.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
https://litmus.mozilla.org/show_test.cgi?id=5017

in-litmus+
Flags: in-litmus? → in-litmus+
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120505 Minefield/3.0b2pre. I verified using stephen's test case in Comment 50 as well as running the sequence of steps in Comment 39.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.