[Tracking Requested - why for this release]: regession Steps To Reproduce: 1. Search any words from SeacrhBar 2. Repeat Step1 until storing enough number of formhistory 3. Select SearchBar and press uparrow downarrow key to open formhistory drop down Actual Results: SearchBar formhistory drop down only shows 7 items The dropdown has no vertical scrollbar Expected Results: SearchBar formhistory drop down should shows 10 items The dropdown should be provided vertical scrollbar

Regression window(m-c) Good: https://hg.mozilla.org/mozilla-central/rev/55c4d770f88b Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140730035505 Bad: https://hg.mozilla.org/mozilla-central/rev/88b5980fb105 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140730045205 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=55c4d770f88b&tochange=88b5980fb105 Regression window(fx) Good: https://hg.mozilla.org/integration/fx-team/rev/94e41f59be31 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140729110404 Bad: https://hg.mozilla.org/integration/fx-team/rev/54d57bd38f51 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140729113008 Pushlog: http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=94e41f59be31&tochange=54d57bd38f51 Regressed by: 54d57bd38f51 Matthew Noorenberghe — Bug 1007979 - Refactor nsSearchSuggestions into a reusable JSM. r=adw Original JSM by mconnor.

3. Select SearchBar and clear then press uparrow downarrow key to open formhistory drop down

This is annoying regression!. Before uplift to Beta from Aurora, the offending patch of Bug 1007979 should be backed out.

Tracking because we are doing a lot on this topic in the 33 release and we want something clean. Mattn, can you help here? Thanks

[Tracking Requested - why for this release]:

Before bug 1007979, SuggestAutoComplete in nsSearchSuggestions.js didn't restrict the number of form history results it returned *when there was no remote request for search suggestions*, i.e., when you press the down key in the search bar: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchSuggestions.js?rev=2520c5e3bf83#169 It also didn't restrict the number when there was a remote request but it timed out. It only restricted the number, to seven, when remote suggestions were successfully retrieved: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchSuggestions.js?rev=2520c5e3bf83#336 Bug 1007979 made it so that the number is always restricted to whatever the client requests: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/SearchSuggestionController.jsm?rev=8cb9028743ac#203 It's a change in behavior but I'm not sure it can be called a bug. Not sure it warrants tracking. The old behavior did end up limiting the number of form history results to ten because of the maxrows attribute set on the search bar's xul:textbox: http://mxr.mozilla.org/mozilla-central/source/browser/components/search/content/search.xml?rev=bcf55557a6b4#39 Somehow that value gets filtered down to the xul:tree that's built inside the popup.

Going to go out on a limb here and say this is not of major importance.

At least, Number of history 7 is too short. All input data should be retrievable. Why Mozilla break the existing feature again and again and again ....?

Except if a low-risk fix comes and it is trivial, wontfix for 33.

The offending bug should be backed out before beta34 ship.

I agree with Drew (comment 6). This is a change in behaviour but I think a minor one. I'm not convinced that showing 7 items instead of 10 warrants tracking or a backout for bug 1007979. I'm dropping tracking for 34 and 35 but am happy to take a fix if it is determined that Firefox really should show 10 items. Alice0775 White - Can you please try to further explain why you think this is a major regression? Why is 7 items too short of a list in this case?

(In reply to Lawrence Mandel [:lmandel] from comment #11) > I agree with Drew (comment 6). This is a change in behaviour but I think a > minor one. I'm not convinced that showing 7 items instead of 10 warrants > tracking or a backout for bug 1007979. I'm dropping tracking for 34 and 35 > but am happy to take a fix if it is determined that Firefox really should > show 10 items. > > Alice0775 White - Can you please try to further explain why you think this > is a major regression? Why is 7 items too short of a list in this case? I used the SearchBar very often. The form history is very useful especially who is IME user. The suggestion and autocomplete is not function very well for IME user, because these function does not work during conversion by IME. The form history is the only means to call a past input phrase for IME user. Therefore, 7 is too short. In Firefox 24, the drop down are 100-200 history available. And I sort them alphabetically by using about:config. This is very very useful. And I can also found several same complaints on 2ch(big online board). 383 名前：名無しさん＠お腹いっぱい。[] 投稿日：2014/08/30(土) 01:55:39.81 ID:h9zoeHEi0 auroraなんだけど検索窓の履歴が7個しか表示されなくなったんですが 仕様変更ですか？about:configから戻せますか？ 975 名前：名無しさん＠お腹いっぱい。[sage] 投稿日：2014/10/03(金) 11:54:14.02 ID:L1w8iqyz0 http://www.07ch.net/up2/src/lena12722.png ▼ボタンを押したら、いつのまにか検索履歴がたった7つしか表示されなくなったのですが、 以前みたいに全部表示させるにはどうすればいいでしょうか？

ni Bryan to bring this use case to his attention. I would be happy to see this change in behaviour reverted if that is in the best interest of our IME users. I have previously flagged this bug for the Firefox desktop backlog.

I think this only happens when there aren't remote results. I didn't realize that we didn't honour the old cap of 7 when there weren't remote results before my patch so I made it always cap to 7 for local results. http://hg.mozilla.org/mozilla-central/annotate/fe3a7fa0732f/toolkit/components/search/nsSearchSuggestions.js#l461

(In reply to Matthew N. [:MattN] (behind on reviews - focused on Loop) from comment #14) > I think this only happens when there aren't remote results. I didn't realize > that we didn't honour the old cap of 7 when there weren't remote results > before my patch so I made it always cap to 7 for local results. > > http://hg.mozilla.org/mozilla-central/annotate/fe3a7fa0732f/toolkit/ > components/search/nsSearchSuggestions.js#l461 This is about form-history doropdown. Not Suggestion.

It's the same dropdown for history suggestions and remote suggestions. They are both suggestions.

I believe the added tests are possibly redundant (given the existing tests I had to modify) and also might not cover all edge cases (lack of remote results for different reasons), but I think I'm OK with that.

This was broken in two places!

Comment on attachment 8506550 [details] [diff] [review] working patch Review of attachment 8506550 [details] [diff] [review]: ----------------------------------------------------------------- Looks like this works fine with about:home and about:newtab since they have |const MAX_DISPLAYED_SUGGESTIONS = 6;|. This will return us to the old behaviour.

Windows try builds failed due to a bug in bug 1041516's patch, repushed Windows only: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0d108509e2db

Comment on attachment 8506550 [details] [diff] [review] working patch Approval Request Comment [Feature/regressing bug #]: bug 1007979 [User impact if declined]: behavior change described here and in bug 1060675 (this also mostly fixes that bug) [Describe test coverage new/current, TBPL]: the new module from bug 1007979 is very well-tested, test coverage is expanded by this patch [Risks and why]: very low risk of breaking anything given test coverage and simplicity of the change, but if we were to break something, it would be isolated to the search suggestions functionality (search bar, newtab, about:home) [String/UUID change made/needed]: none

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #23) > [User impact if declined]: behavior change described here and in bug 1060675 > (this also mostly fixes that bug) I meant bug 1060846.

Comment on attachment 8506550 [details] [diff] [review] working patch Beta+ Aurora+

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a29843ef488 https://hg.mozilla.org/releases/mozilla-beta/rev/a963eab53a09

Verified that the Search Bar shows all the previous searches when pressing down arrow key in a clear field using Firefox 34 beta 3 (20141023111813), latest Aurora 35.a02 (20141024004006) and latest Nightly 36.0a1 (20141024030200) under Win 7 64-bit, Ubuntu 14.04 32-bit and Mac OSX 10.9.5. Possibles issues: - Having 10 searches in history that starts with "a", and then typing "a" in Search Bar will display only 7 suggestions - If search fields in about:newtab and about:home are empty, 6 suggestions will appear when pressing down arrow key. However, only two suggestions are shown after starting typing. Gavin, is this what you meant in comment 25?

Thanks for the testing! (In reply to Petruta Rasa [QA] [:petruta] from comment #28) > Possibles issues: > - Having 10 searches in history that starts with "a", and then typing "a" in > Search Bar will display only 7 suggestions Along with other search suggestions, right? It is expected that we only show at most 7 history results when search suggestions (from the search provider) are also displayed. > - If search fields in about:newtab and about:home are empty, 6 suggestions > will appear when pressing down arrow key. However, only two suggestions are > shown after starting typing. This is currently expected, and covered by other bugs.

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #29) > Along with other search suggestions, right? It is expected that we only show > at most 7 history results when search suggestions (from the search provider) > are also displayed. That's correct. Thanks a lot! Marking as verified based on above comments.

(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #29) > Along with other search suggestions, right? It is expected that we only show > at most 7 history results when search suggestions (from the search provider) > are also displayed. That's correct. Thanks a lot! Marking as verified based on above comments.