Closed
Bug 1060846
Opened 10 years ago
Closed 10 years ago
Form history dropdown shows only 2 items in search field of about:home/about:newtab
Categories
(Firefox :: Search, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox33 | --- | unaffected |
firefox34 | --- | verified |
firefox35 | --- | verified |
People
(Reporter: alice0775, Assigned: Gavin)
References
Details
(Keywords: regression)
[Tracking Requested - why for this release]: [Tracking Requested - why for this release]: Steps To Reproduce: Steps To Reproduce: 1. Open about:home and search from search filed 2. Repeat step2 more than seven times with different search term) 3. Open about:home, focus search filed and press downarrow key Actual Results: Form history doropdown shows only 2 items Expected Results: Form history doropdown should show 6 items and vertical scrollbar if necessary
![]() |
Reporter | |
Comment 1•10 years ago
|
||
Regression window(m-c) Good: https://hg.mozilla.org/mozilla-central/rev/6551323d96e6 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140801134649 Bad: https://hg.mozilla.org/mozilla-central/rev/eed9fe35a00d Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140830030204 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6551323d96e6&tochange=eed9fe35a00d Regression window(fx) Good: https://hg.mozilla.org/integration/fx-team/rev/f08fb8275e5b Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140801112450 Bad: https://hg.mozilla.org/integration/fx-team/rev/fe53a2b0e7d0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140801120154 Pushlog: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=f08fb8275e5b&tochange=fe53a2b0e7d0 Regressed by: Bug 612453 and Bug 1028985
Summary: Form history doropdown shows only 2 items in search field of about:home/about:newtab → Form history dropdown shows only 2 items in search field of about:home/about:newtab
Updated•10 years ago
|
Comment 2•10 years ago
|
||
Thanks for filing. SearchSuggestionController limits the number of form history entries here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/SearchSuggestionController.jsm?rev=8cb9028743ac#203 That should probably be changed to return maxLocalResults + maxRemoteResults when the search string passed to fetch is empty. (The search string is empty when you press the down key in about:home/newtab's search input.) Bug 1046943 is kind of related in that it also changes maxLocalResults and maxRemoteResults to be more like hints than hard limits. Marking this as blocking bug 1007979 as well, which really isn't fair because it's more bug 612453's fault, but as I mentioned the relevant code is in SearchSuggestionController.jsm.
Comment 3•10 years ago
|
||
Circling back to this -- bug 1066787 is the long-term plan for suggestions on about:home/newtab, so I will add a dependency on it. When that is fixed, about:home/newtab will show the same number of form history results as the search bar does (currently seven, but see bug 1060675).
Depends on: 1066787
Comment 4•10 years ago
|
||
Looks like accidental flagging on esr24, clearing.
status-firefox-esr24:
affected → ---
tracking-firefox-esr24:
? → ---
Comment 5•10 years ago
|
||
Bryan - 2 items for search history seems pretty limiting. What's your take on the importance of fixing this bug?
Flags: needinfo?(clarkbw)
Flags: firefox-backlog?
Comment 6•10 years ago
|
||
Yeah, this isn't good. Would be nice if those two history search terms were the most recent, doesn't necessarily appear so from my testing. We should get this fixed but it doesn't feel like a blocker to me. You can still access the full list from the search input so we have a work around for now.
Flags: needinfo?(clarkbw)
Comment 7•10 years ago
|
||
Based on comment 6, I'm going to drop tracking for 34. However, I would be happy to take a safe patch early in beta if the desktop team wants to accept this into the backlog and prioritize it.
status-firefox35:
--- → affected
tracking-firefox34:
+ → ---
Comment 8•10 years ago
|
||
(In reply to Drew Willcoxon :adw (Away 9/29–10/14) from comment #2) > That should probably be changed to return maxLocalResults + maxRemoteResults > when the search string passed to fetch is empty. (The search string is > empty when you press the down key in about:home/newtab's search input.) This is also necessary when search suggestions are disabled by the user via the context menu in the search box. See bug 612453 comment 105.
Comment 9•10 years ago
|
||
Do you have remote (search provider) suggestions disabled? See browser.search.suggest.enabled. If so, a problem is that https://mxr.mozilla.org/mozilla-central/source/browser/modules/ContentSearch.jsm?rev=f7e5136269fa&mark=252#251 should take that pref into account.
Flags: needinfo?(alice0775)
Comment 10•10 years ago
|
||
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #6) > Yeah, this isn't good. Would be nice if those two history search terms were > the most recent, doesn't necessarily appear so from my testing. It's sorted by frecency.
![]() |
Reporter | |
Comment 11•10 years ago
|
||
(In reply to Matthew N. [:MattN] (behind on reviews - focused on Loop) from comment #9) > Do you have remote (search provider) suggestions disabled? See > browser.search.suggest.enabled. > > If so, a problem is that > https://mxr.mozilla.org/mozilla-central/source/browser/modules/ContentSearch. > jsm?rev=f7e5136269fa&mark=252#251 should take that pref into account. This is about form-history drop down. Not suggestion. not related browser.search.suggest.enabled thing.
Flags: needinfo?(alice0775)
Comment 12•10 years ago
|
||
It is related because we sometimes take different code paths depending on whether remote results are enabled.
Assignee | ||
Comment 13•10 years ago
|
||
My patch in bug 1060675 produces the expected results for comment 0 (6 suggestions appear with those STR).
Depends on: 1060675
Assignee | ||
Comment 14•10 years ago
|
||
Though I'm not sure I understand why _onMessageGetSuggestions's maxRemoteResults/maxLocalResults behavior depends on whether suggestions are provided. Shouldn't we just remove that and rely on SearchSuggestionController's default values all the time?
Comment 15•10 years ago
|
||
I think the reason was that we only show 6 results total on the about page (not ideal either) and so the default of maxLocalResults: 7 would mean no remote results would be visible in some cases. One of the reasons for only 6 results total is because we would have to do more work to deal with adding scrolling to the result popup and adjusting its size based on the viewport since the dropdown is just HTML and must be constrained by the browser window content area. Bug 1066787 would do this for free.
Assignee | ||
Comment 16•10 years ago
|
||
OK, let's call this fixed by bug 1060675 then. The "we should show a scrollbar" part should be split off to a new bug as needed.
Assignee: nobody → gavin.sharp
Status: NEW → RESOLVED
Closed: 10 years ago
No longer depends on: 1066787
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Updated•10 years ago
|
Iteration: --- → 36.2
Flags: qe-verify?
Flags: firefox-backlog?
Flags: firefox-backlog+
Assignee | ||
Updated•10 years ago
|
Flags: qe-verify? → qe-verify+
Updated•10 years ago
|
QA Contact: petruta.rasa
You need to log in
before you can comment on or make changes to this bug.
Description
•