Closed Bug 1060675 Opened 5 years ago Closed 5 years ago

SearchBar formhistory drop down only shows 7 items when only searching form history

Categories

(Firefox :: Search, defect)

33 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 36
Iteration:
36.1
Tracking Status
firefox32 --- unaffected
firefox33 + wontfix
firefox34 --- verified
firefox35 --- verified
firefox36 --- verified

People

(Reporter: alice0775, Assigned: Gavin)

References

Details

(Keywords: regression, uiwanted)

Attachments

(2 files, 1 obsolete file)

Attached image screenshot
[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.
Blocks: 1007979
Summary: SearchBar formhistory drop down only shows 7 items and no vertical scrollbar in Aurora33.0a2 → SearchBar formhistory drop down only shows 7 items and no vertical scrollbar in Nightly34.0a1 and Aurora33.0a2
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.
Severity: normal → major
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
Flags: needinfo?(MattN+bmo)
[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.
Flags: needinfo?(MattN+bmo)
Going to go out on a limb here and say this is not of major importance.
Severity: major → normal
OS: Windows 7 → All
Hardware: x86_64 → All
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 ....?
Keywords: uiwanted
Flags: firefox-backlog?
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?
Flags: needinfo?(alice0775)
(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つしか表示されなくなったのですが、
以前みたいに全部表示させるにはどうすればいいでしょうか?
Flags: needinfo?(alice0775)
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.
Flags: needinfo?(clarkbw)
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
Summary: SearchBar formhistory drop down only shows 7 items and no vertical scrollbar in Nightly34.0a1 and Aurora33.0a2 → SearchBar suggestions only shows 7 items when only searching form history
(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.
Summary: SearchBar suggestions only shows 7 items when only searching form history → SearchBar formhistory drop down only shows 7 items when only searching form history
It's the same dropdown for history suggestions and remote suggestions. They are both suggestions.
Attached patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #8506540 - Flags: review?(MattN+bmo)
Flags: qe-verify+
Flags: needinfo?(clarkbw)
Flags: in-testsuite+
Flags: firefox-backlog?
Flags: firefox-backlog+
Attached patch working patchSplinter Review
This was broken in two places!
Attachment #8506540 - Attachment is obsolete: true
Attachment #8506540 - Flags: review?(MattN+bmo)
Attachment #8506550 - Flags: review?(MattN+bmo)
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.
Attachment #8506550 - Flags: review?(MattN+bmo) → review+
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
Attachment #8506550 - Flags: approval-mozilla-beta?
Attachment #8506550 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e20104cdd87e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Iteration: --- → 36.1
QA Contact: petruta.rasa
(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+
Attachment #8506550 - Flags: approval-mozilla-beta?
Attachment #8506550 - Flags: approval-mozilla-beta+
Attachment #8506550 - Flags: approval-mozilla-aurora?
Attachment #8506550 - Flags: approval-mozilla-aurora+
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?
Flags: needinfo?(gavin.sharp)
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.
Flags: needinfo?(gavin.sharp)
(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.
Status: RESOLVED → VERIFIED
(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.
You need to log in before you can comment on or make changes to this bug.