Closed
Bug 1162144
Opened 10 years ago
Closed 9 years ago
Add link to search settings to AwesomeBar
Categories
(Firefox :: Search, defect, P1)
Tracking
()
People
(Reporter: kev, Assigned: mossop)
References
Details
(Whiteboard: [suggestions][fxsearch])
User Story
As a user, I will be able to control whether text entered in the location bar is shared with a search provider to receive suggested searches to provide me with better control over what information I share with third parties.
Attachments
(3 files, 4 obsolete files)
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)
Updated•10 years ago
|
Rank: 5
Flags: firefox-backlog+
Priority: -- → P1
Updated•9 years ago
|
Whiteboard: [fxsearch][searchsuggestions] → [suggestions][fxsearch]
Comment 1•9 years ago
|
||
Steve - attach design :)
Assignee: nobody → dtownsend
Iteration: --- → 41.1 - May 25
Flags: needinfo?(shorlander)
Comment 2•9 years ago
|
||
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.
Flags: needinfo?(shorlander) → needinfo?(dtownsend)
Assignee | ||
Comment 4•9 years ago
|
||
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
Flags: needinfo?(shorlander)
Comment 5•9 years ago
|
||
(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
Flags: needinfo?(shorlander)
Assignee | ||
Comment 6•9 years ago
|
||
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?
Attachment #8609651 -
Flags: review?(dao)
Comment 7•9 years ago
|
||
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.
Attachment #8609651 -
Flags: review?(dao) → review-
Updated•9 years ago
|
Status: NEW → ASSIGNED
Flags: qe-verify?
Updated•9 years ago
|
Iteration: 41.1 - May 25 → 41.2 - Jun 8
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8609651 -
Attachment is obsolete: true
Attachment #8611432 -
Flags: review?(dao)
Comment 9•9 years ago
|
||
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
Attachment #8611432 -
Flags: review?(dao) → review-
Comment 10•9 years ago
|
||
Assignee | ||
Comment 11•9 years ago
|
||
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?
Flags: needinfo?(shorlander)
Assignee | ||
Comment 12•9 years ago
|
||
(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•9 years ago
|
||
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?
Updated•9 years ago
|
Iteration: 41.2 - Jun 8 → 41.3 - Jun 29
Assignee | ||
Comment 14•9 years ago
|
||
(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
Assignee | ||
Comment 15•9 years ago
|
||
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.
Attachment #8621318 -
Flags: review?(dao)
Comment 16•9 years ago
|
||
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.
Attachment #8611432 -
Attachment is obsolete: true
Attachment #8612242 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8621318 -
Flags: review?(dao) → review-
Assignee | ||
Comment 17•9 years ago
|
||
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.
Attachment #8621318 -
Attachment is obsolete: true
Attachment #8623221 -
Flags: review?(dao)
Comment 18•9 years ago
|
||
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.
Attachment #8623221 -
Flags: review?(dao) → review+
Assignee | ||
Comment 20•9 years ago
|
||
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.
Flags: needinfo?(shorlander)
Comment 21•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment 22•9 years ago
|
||
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
Updated•9 years ago
|
Flags: needinfo?(shorlander)
Comment 23•9 years ago
|
||
(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•9 years ago
|
||
(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•9 years ago
|
||
(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.
Flags: needinfo?(shorlander)
Updated•9 years ago
|
Flags: qe-verify? → qe-verify+
QA Contact: petruta.rasa
Comment 26•9 years ago
|
||
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.
Status: RESOLVED → VERIFIED
Comment 27•9 years ago
|
||
(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•9 years ago
|
||
(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?
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 29•9 years ago
|
||
(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.
Flags: needinfo?(dtownsend)
You need to log in
before you can comment on or make changes to this bug.
Description
•