1162144 - Add link to search settings to AwesomeBar

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Kev Needham [:kev]]

Allow users to disable Search Suggests from the Awesomebar (e.g. provide a method where users can get at the search settings, for the purposes of disabling suggests, from the awesomebar)

Comment 1

[:shell escalante]

Steve - attach design :)

Comment 2

[Stephen Horlander [:shorlander]]

Attachment: Change Search Settings Button - 01

The Search Settings button is part of the footer which will include the one-off search buttons; e.g. http://people.mozilla.org/~shorlander/Flare/Flare-Search-Unification-%28OSX%29-03.png

We can add the footer and the Search Settings button independently of the rest.

Comment 3

[Dave Townsend [:mossop]]

Looks good, thanks

Comment 4

[Dave Townsend [:mossop]]

Seems like there is some gradient in the image. Can you confirm the following colours:

Footer bar background
Footer bar top border
Button hover
Button active

Comment 5

[Stephen Horlander [:shorlander]]

(In reply to Dave Townsend [:mossop] from comment #4)
> Seems like there is some gradient in the image. Can you confirm the
> following colours:
> 
> Footer bar background
> Footer bar top border
> Button hover
> Button active

Sorry, I left the translucency in the mockup:

Footer bar background — hsla(210,4%,10%,.07); 
Footer bar top border — hsla(210,4%,10%,.14);
Button hover — hsla(210,4%,10%,.07);
Button active — hsla(210,4%,10%,.12);

I pulled these from the panel footer for consistency:
https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUIOverlay.inc.css#488

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch

Turns out to be pretty straightforward. There isn't a lot in the shared browser.inc stylesheet, should I be putting the rules in the per-platform stylesheets?

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8609651 [details] [diff] [review]
patch

>+      <hbox class="urlbar-search-footer" flex="1" align="stretch" pack="end">
>+        <button class="urlbar-search-settings" label="&changeSearchSettings.button;"

You can use id instead of class since these aren't supposed to be used elsewhere.

>+.urlbar-search-settings {
>+  -moz-appearance: none;
>+  min-height: 32px;
>+  margin: 0;
>+  color: #666;
>+  padding: 0 20px;

Please add vertical padding and get rid of the min-height.

Behind the hsla background, there's a native background which isn't necessarily white, so instead of #666 you should use a platform color for the text, such as graytext.

> Turns out to be pretty straightforward. There isn't a lot in the shared
> browser.inc stylesheet, should I be putting the rules in the per-platform
> stylesheets?

Yes, browser.inc is only used for defines.

Comment 8

[Dave Townsend [:mossop]]

Attachment: patch

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8611432 [details] [diff] [review]
patch

* I can't seem to click the button. When I try to, the popup just closes

* the button seems inaccessible by keyboard

* the styling is broken on Linux (screenshot coming), likely affects Windows too

Comment 10

[Dão Gottwald [:dao]]

Attachment: screenshot

Comment 11

[Dave Townsend [:mossop]]

Stephen, how do you want keyboard selection to work for this button? In unified autocomplete land when you type the urlbar has focus and there is always at least one item in the dropdown list highlighted. Tabbing keeps focus on the url bar and instead moves up and down through the list. I guess we could make tabbing at the end of the list drop focus from the url bar, deselect the list item and then focus the settings button, I'm wary that that might break assumptions that there is always one list item selected though. Even then what happens after you press tab again?

Comment 12

[Dave Townsend [:mossop]]

(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 8611432 [details] [diff] [review]
> patch
> 
> * I can't seem to click the button. When I try to, the popup just closes

This is weird. Seems to be ok on windows and OSX. On Linux I can use the button the first time the awesomebar shows but not after that.

> * the button seems inaccessible by keyboard

Depending on when UX decide what the right thing is here it might make sense to take this without keyboard accessibility at first.

> * the styling is broken on Linux (screenshot coming), likely affects Windows
> too

There's a simple fix for this, I forgot the other platforms include a default border for buttons.

Comment 13

[Marco Bonardo [:mak]]

Just a question, the toggles to control what's suggested in Autocomplete are under Privacy, so I'd expect a Search Suggestions toggle there. But here we are linking Search prefs to change the engine.
I suppose we are going to have both this link and a link to privacy settings (or a direct toggle for suggestions, duped under privacy), implemented through bug 959567?

Comment 14

[Dave Townsend [:mossop]]

(In reply to Marco Bonardo [::mak] from comment #13)
> Just a question, the toggles to control what's suggested in Autocomplete are
> under Privacy, so I'd expect a Search Suggestions toggle there. But here we
> are linking Search prefs to change the engine.
> I suppose we are going to have both this link and a link to privacy settings
> (or a direct toggle for suggestions, duped under privacy), implemented
> through bug 959567?

I believe that this is the intent yes, I'll make sure Kev is on it though

Comment 15

[Dave Townsend [:mossop]]

Attachment: patch

For now this defers on the keyboard accessibility piece and that makes making the button work on linux easy since it was the urlbar losing focus that was causing the popup to close too soon. This adds a test too.

Comment 16

[Dão Gottwald [:dao]]

Attachment: screenshot

Closer but still not quite there. You'll need to remove the button's default background color, but be aware that this might make the contrast between the gray background and the text color too small.

Comment 17

[Dave Townsend [:mossop]]

Attachment: patch

Updated with transparent backgrounds on windows and linux. Windows looked fine, linux didn't as you suspected. I used the color #666 for the text there, that matches that of the same button in the search bar drop-down so I assume that is ok. I don't know if there is a platform colour that would work here.

Comment 18

[Dão Gottwald [:dao]]

Comment on attachment 8623221 [details] [diff] [review]
patch

>--- a/browser/themes/linux/browser.css
>+++ b/browser/themes/linux/browser.css

>+#urlbar-search-footer {
>+  border-top: 1px solid hsla(210,4%,10%,.14);
>+  background-color: hsla(210,4%,10%,.07);
>+}
>+
>+#urlbar-search-settings {
>+  -moz-appearance: none;
>+  -moz-user-focus: ignore;
>+  color: #666;
>+  margin: 0;
>+  border: 0;
>+  padding: 8px 20px;
>+  background: transparent;
>+}

#666 isn't guaranteed to be legible here. Let's just use 'color: inherit' on Linux for now.

I assume you'll only want to land this after the next merge since browser.urlbar.suggest.searches is only true for NIGHTLY_BUILD right now...

I think we should also consider linking to about:preferences#privacy since that's where all the other location prefs are.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/9976683b3e5a

Comment 20

[Dave Townsend [:mossop]]

Landed with that color change. I left the pref pane alone for now, shorlander was talking about maybe moving the suggestions settings around so we'll maybe adjust after that.

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9976683b3e5a

Comment 22

[Marco Bonardo [:mak]]

I think this footer is a little bit too prominent, should it be the same height as the one in the search bar? It is quite taller

Comment 23

[Tim Nguyen :ntim]

(In reply to Marco Bonardo [::mak] from comment #22)
> I think this footer is a little bit too prominent, should it be the same
> height as the one in the search bar? It is quite taller

It's consistent with the other panel footers (bookmarks panel, history panel).

Comment 24

[Marco Bonardo [:mak]]

(In reply to Tim Nguyen [:ntim] from comment #23)
> (In reply to Marco Bonardo [::mak] from comment #22)
> > I think this footer is a little bit too prominent, should it be the same
> > height as the one in the search bar? It is quite taller
> 
> It's consistent with the other panel footers (bookmarks panel, history
> panel).

then it's the searchbar footer that it's not consistent? From the mockups the locationbar and searchbar footers look the same.
Also the mock-up attached in this same bug states it should be 32px
https://bug1162144.bugzilla.mozilla.org/attachment.cgi?id=8607550
Mine is like 41px

Comment 25

[Stephen Horlander [:shorlander]]

(In reply to Marco Bonardo [::mak] from comment #22)
> I think this footer is a little bit too prominent, should it be the same
> height as the one in the search bar? It is quite taller

Maybe? It's a little hard to evaluate in its current state since it was designed with the search tiles and the rest of the panel changes in mind. But I agree it feels out of place right now.

Comment 26

[Petruta Horea [:phorea], Desktop QA]

Checked with latest Nightly 42.0a1 2015-08-02 under Mac OS X 10.10.4, Win 7 64-bit, Ubuntu 12.04 32-bit using OS's default themes.

* The Change Search Settings button from Location Bar drop-down, opens about:preferences#search page as per design, but instead it should open about:preferences#privacy - Location Bar as stated by User Story and comment 0. (also mentioned in above comments).

* The footer's height for me is 38px on Windows and 33px on Mac.

Considering that bug 959567 will fix these points, I'm marking this bug as verified.

Comment 27

[Marco Bonardo [:mak]]

(In reply to Petruta Rasa [QA] [:petruta] from comment #26)
> * The Change Search Settings button from Location Bar drop-down, opens
> about:preferences#search page as per design, but instead it should open
> about:preferences#privacy - Location Bar as stated by User Story and comment
> 0. (also mentioned in above comments).

Nope, it should open "search", always. the opt-in/out will open "privacy".

> * The footer's height for me is 38px on Windows and 33px on Mac.
> 
> Considering that bug 959567 will fix these points, I'm marking this bug as
> verified.

no, that bug won't touch anything that has been added here.

Comment 28

[Luís Miguel [:quicksaver] (not reading bugmail)]

(Not sure who I should be asking, so I'm starting with Dave as the assignee for this bug. If you're not the right person, can you please recommend someone?)

I'm planning to add a few customization features for the location bar into my add-on, one of which includes adding the same "change search settings" button into Firefox release/beta and modifying nigthly's existing one for consistency. *BUT* clicking it sends you to the add-on's preferences tab, which has its own set of options of course, which in turn has another button to redirect to Firefox's options-search pane; this will happen in all Firefox versions.

Should I:
- keep the telemetry counting when clicking the button in that case?
- change the argument "urlbar" into something else like "urlbar-awesomer"? 
- do either of those *only* in Firefox release/beta or *also* in nightly builds? 
- take the telemetry call out completely in every case because it's being added/modified by an add-on?

What's the best way to not screw up telemetry results by users of the add-on?

Comment 29

[Dave Townsend [:mossop]]

(In reply to Luís Miguel [:quicksaver] from comment #28)
> (Not sure who I should be asking, so I'm starting with Dave as the assignee
> for this bug. If you're not the right person, can you please recommend
> someone?)
> 
> I'm planning to add a few customization features for the location bar into
> my add-on, one of which includes adding the same "change search settings"
> button into Firefox release/beta and modifying nigthly's existing one for
> consistency. *BUT* clicking it sends you to the add-on's preferences tab,
> which has its own set of options of course, which in turn has another button
> to redirect to Firefox's options-search pane; this will happen in all
> Firefox versions.
> 
> Should I:
> - keep the telemetry counting when clicking the button in that case?
> - change the argument "urlbar" into something else like "urlbar-awesomer"? 
> - do either of those *only* in Firefox release/beta or *also* in nightly
> builds? 
> - take the telemetry call out completely in every case because it's being
> added/modified by an add-on?
> 
> What's the best way to not screw up telemetry results by users of the add-on?

I have no idea, I'd try asking the #telemetry folks in IRC, but I'm guessing that unless you have a smash hit on your hands it won't be enough to sway results much.