"Show search suggestions ahead of browsing history in address bar results" checkbox shouldn't be in the Search Bar groupbox

VERIFIED FIXED in Firefox 59

Status

()

defect
P1
normal
VERIFIED FIXED
Last year
Last year

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 unaffected, firefox59+ verified, firefox60+ verified)

Details

Attachments

(1 attachment)

Assignee

Description

Last year
This checkbox is about the address bar, so putting it in the Search Bar groupbox hardly makes any sense. There are other checkboxes related to search suggestions in the Default Search Engine groupbox where this fits much better.
Assignee

Updated

Last year
Attachment #8945023 - Flags: ui-review?(shorlander)
Javaun, you posted the mockup for this UI in bug 1426216 comment 0.  Could you please comment on Dao's change?

(In reply to Dão Gottwald [::dao] from comment #0)
> This checkbox is about the address bar, so putting it in the Search Bar
> groupbox hardly makes any sense. There are other checkboxes related to
> search suggestions in the Default Search Engine groupbox where this fits
> much better.
Flags: needinfo?(jmoradi)
Assigning a priority to get this off the triage list.
Priority: -- → P3
Assignee

Comment 5

Last year
[Tracking Requested - why for this release]:
We should fix this before shipping and advertising this feature in 59 (bug 1426216 comment 29)
If we want to fix this for 59, it probably can't be P3 (we would need to land and verify a fix in the next couple of weeks).
Assignee

Updated

Last year
Priority: P3 → P1
Comment on attachment 8945023 [details]
Bug 1432716 - Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander

Looks good to me. It definitely makes more sense with that grouping. Thank you!


Just a note: This does do interesting things with the visual display of pref nesting/dependency. If we followed the pattern it would be indented, but indenting three rows of single line prefs doesn't look great.
Attachment #8945023 - Flags: ui-review?(shorlander) → ui-review+
Drew, I think UX is more Stephen's strong suit than Javaun's, with all due respect, so let's take the ui-r+ and continue the review process. Thanks!
Flags: needinfo?(jmoradi)

Comment 9

Last year
mozreview-review
Comment on attachment 8945023 [details]
Bug 1432716 - Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander

https://reviewboard.mozilla.org/r/215222/#review224650
Attachment #8945023 - Flags: review?(adw) → review+

Comment 10

Last year
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98b4549200aa
Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander r=adw
Comment hidden (mozreview-request)

Comment 13

Last year
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/58ad0c23fc93
Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander r=adw

Comment 14

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/58ad0c23fc93
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Assignee

Updated

Last year
Flags: qe-verify+
Keywords: verifyme
Assignee

Comment 15

Last year
Comment on attachment 8945023 [details]
Bug 1432716 - Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander

Approval Request Comment
[Feature/Bug causing the regression]: bug 1426216
[User impact if declined]: search suggestion checkboxes are confusing
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: STR: open about:preferences#search and play with the checkboxes
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: not much
[Why is the change risky/not risky?]: mostly moving code around. The only slightly tricky part is that I had to adjust the JS logic so that this checkbox would get disabled depending on the other checkboxes.
[String changes made/needed]: /
Attachment #8945023 - Flags: approval-mozilla-beta?
Comment on attachment 8945023 [details]
Bug 1432716 - Group the "Show search suggestions ahead of browsing history in address bar results" checkbox with other search suggestion checkboxes. ui=shorlander

Makes sense, let's uplift this for beta 10. 
We should verify it in beta.
Attachment #8945023 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have verified this on Firefox 60.0a1 (BuildId:20180213100127) using Windows 10 64bit, Ubuntu 16.04 64bit and macOS 10.13. 

Testing was performed around the "Show search suggestions ahead of browsing history in address bar results" button, checking both functionality over the address bar and if the button gets successfully disabled/enabled in accordance with the other checkboxes.

Leaving a ni on myself just for a reminder to verify this in 59.b10.
Status: RESOLVED → VERIFIED
Flags: needinfo?(emil.ghitta)
Assignee

Updated

Last year
Keywords: verifyme
Verified using Firefox 59.0b10 (BuildId:20180215111455) on Windows 10 64bit, macOS 10.13 and Ubuntu 16.04 64bit.
Flags: qe-verify+
Flags: needinfo?(emil.ghitta)
You need to log in before you can comment on or make changes to this bug.