Closed Bug 1066794 Opened 6 years ago Closed 6 years ago

Make the search suggestions popup on about:home/about:newtab more consistent with the main search bar's popup for 33

Categories

(Firefox :: General, defect)

defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 35
Iteration:
35.2
Tracking Status
firefox33 + verified
firefox34 + verified
firefox35 --- verified

People

(Reporter: adw, Assigned: adw)

References

Details

Attachments

(2 files, 1 obsolete file)

[Tracking Requested - why for this release]:

Gavin, Phillip, and I talked over email about this.  For 33, we want to land some simple changes to make the search suggestions popup on about:home/about:newtab more consistent with the main search bar's popup.  Longer term, we want to do bug 1054951 and its dependents.

Changes are:

* Remove the blue text color
* Remove the bold text
* Add a separator line between form history and search suggestions

All of that should be fairly simple by changing the CSS here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/searchSuggestionUI.css

There may be some JS involved to tell the CSS where to draw the separator line between the two types of results.  That would be here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/searchSuggestionUI.js
Flags: qe-verify+
Flags: firefox-backlog+
Assignee: nobody → adw
Status: NEW → ASSIGNED
Iteration: --- → 35.2
Depends on: 1060888
Attached patch patch (obsolete) — Splinter Review
I don't think there's a way to draw the border without adding JS like this.
Attachment #8490272 - Flags: review?(MattN+bmo)
Attached image screenshot
Comment on attachment 8490272 [details] [diff] [review]
patch

>+++ b/browser/base/content/searchSuggestionUI.css
>+.searchSuggestionRow.remoteBorder > td {
>+  border-top: 1px solid GrayText;
Using the existing .remote class, you could do this (although I have no idea its performance impact):

/* Add a separating border above remote entries.. */
.searchSuggestionRow.remote > td {
  border-top: 1px solid GrayText;
}

/* .. but only for the first one */
.searchSuggestionRow.remote ~ .remote > td {
  border-top: none;
}
Attached patch CSS-only patchSplinter Review
Nice!  That's close but not quite what we want, since it draws the border even when there's no form history.  I tested this with form history only, remote results only, and both, and it looks good.  Screenshot is the same as the one I already posted.
Attachment #8490272 - Attachment is obsolete: true
Attachment #8490272 - Flags: review?(MattN+bmo)
Attachment #8490307 - Flags: review?(MattN+bmo)
Oh. Nice catch. And today I also learned about bug 854148, which would allow for .searchSuggestionRow:nth-child(1 of .remote), but that would still have incorrectly styled if .remote was the first child (although I guess :not(:first-child) and the + would fix that too.)
Attachment #8490307 - Flags: review?(MattN+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/cda99b263dbb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Approval requests needed.
Flags: needinfo?(adw)
Comment on attachment 8490307 [details] [diff] [review]
CSS-only patch

Approval Request Comment
[Feature/regressing bug #]: search suggestions in about:home/newtab (bug 612453)
[User impact if declined]: search suggestions popup in about:home/newtab will look a little different from the main search bar's popup
[Describe test coverage new/current, TBPL]: manual testing
[Risks and why]: low risk, CSS changes only
[String/UUID change made/needed]: none
Attachment #8490307 - Flags: approval-mozilla-beta?
Attachment #8490307 - Flags: approval-mozilla-aurora?
Flags: needinfo?(adw)
Attachment #8490307 - Flags: approval-mozilla-beta?
Attachment #8490307 - Flags: approval-mozilla-beta+
Attachment #8490307 - Flags: approval-mozilla-aurora?
Attachment #8490307 - Flags: approval-mozilla-aurora+
QA Contact: petruta.rasa
I verified this bug using the following environment:

FF 35
Build Id:20140921030208
OS: Win 7 x64, Ubuntu 12.10 x64, Mac Os X 10.9.5

and I found the following issue:
Only a max of 2 items are above the separator line between from history and search suggestions.
STR:

1. From about:home or about:newtab search for casey kasem, cash converters and castle
2. On about:home or about:newtab type cas 

Issue:
Only 2 items are displayed above the separator line. In the main search's bar popup all 3 entry are displayed above the line.
That's bug 1060846 basically.
Thanks Drew you are correct.

I've verified the bug on the following environment:

FF 33.0b6 Build Id:20140922173023
FF 34 Build Id: 20140922004004

OS: Win 7 x64, Mac Os X 10.9.5, Ubuntu 12.10 x32
You need to log in before you can comment on or make changes to this bug.