first-use tour for new one-off search UI

VERIFIED FIXED in Firefox 34

Status

()

Firefox
Search
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: Gavin, Assigned: mossop)

Tracking

Trunk
Firefox 36
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox34+ verified, firefox35+ verified, firefox36+ verified)

Details

Attachments

(6 attachments, 1 obsolete attachment)

Created attachment 8525414 [details]
mock-up

We will likely want some form of in-product "tour" of the new UI in bug 1088660. We will want this to be triggered when you use the UI, so it probably won't be doable as a UITour but rather something custom for the search bar.
Created attachment 8525415 [details]
better mock-up
(Assignee)

Updated

4 years ago
Assignee: nobody → dtownsend+bugmail
Group: mozilla-employee-confidential
(Assignee)

Updated

4 years ago
Depends on: 1088660
(Assignee)

Comment 2

4 years ago
Created attachment 8525704 [details]
screenshot

Current look
(Assignee)

Comment 3

4 years ago
Created attachment 8525707 [details] [diff] [review]
patch

This is going to require some style and XUL changes once we get final mockups but the mechanics of showing the highlight panels for the first N times (currently 10) all work here.

For now we're punting on keyboard accessibility.
Attachment #8525707 - Flags: review?(felipc)
Comment on attachment 8525707 [details] [diff] [review]
patch

Review of attachment 8525707 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +3306,5 @@
> +    }
> +
> +    // If the highlight has already been show the appropriate number of times
> +    // do nothing.
> +    let count = 11;//Services.prefs.getIntPref(this.countPref);

debugging?
Attachment #8525707 - Flags: review?(felipc) → review+
Comment on attachment 8525707 [details] [diff] [review]
patch

Review of attachment 8525707 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, general comment: One thing I don't see here is an adjusted message for users who already had Yahoo has their default, or who are in locales where the default search provider is _not_ changing. Seems like we should still have this point out the shiny new UI, but we should avoid making them think something changed when it didn't, and may even want to be quite explicit/reassuring to those who will not see a search engine change.

::: browser/app/profile/firefox.js
@@ +425,5 @@
>  #endif
>  
> +// How many times to show the new search highlight
> +pref("browser.search.highlightCount", 10);
> +

That's a lot. But IIRC the intent was to be a little noisy about this change, so I suppose that's accomplished.

::: browser/base/content/browser.xul
@@ +256,5 @@
> +      <hbox class="SearchHighlightContent">
> +        <description class="SearchHighlightText" flex="1">&SearchHighlight1.text;</description>
> +      </hbox>
> +      <hbox class="SearchHighlightFooter" align="center">
> +        <label class="text-link" href="https://www.mozilla.org/">&SearchHighlightLink;</label>

That's not a very descriptive "Learn More" link. Is there (web)content being created for this feature? Followup bug?

@@ +277,5 @@
> +      <hbox class="SearchHighlightContent">
> +        <description class="SearchHighlightText" flex="1">&SearchHighlight2.text;</description>
> +      </hbox>
> +      <hbox class="SearchHighlightFooter" align="center">
> +        <label class="text-link" href="https://www.mozilla.org/">&SearchHighlightLink;</label>

Ditto.

::: browser/locales/en-US/chrome/browser/searchbar.dtd
@@ +6,5 @@
>  <!ENTITY searchEndCap.label             "Search">
> +
> +<!ENTITY SearchHighlight1.title         "New search provider">
> +<!ENTITY SearchHighlight1.text          "Firefox now lets you search easily on your favourite sites.">
> +<!ENTITY SearchHighlight1.page          "1/2">

Nit: I found myself wondering why there was a "one half" floating in the window. Make this "1 of 2" and "2 of 2"?

(Or, really, since this is only a 2-part thing, maybe the numbers just add more confusion than they actually fix. If it was a longer multi-step thing that would seem more important...)

@@ +9,5 @@
> +<!ENTITY SearchHighlight1.text          "Firefox now lets you search easily on your favourite sites.">
> +<!ENTITY SearchHighlight1.page          "1/2">
> +
> +<!ENTITY SearchHighlight2.title         "New search provider">
> +<!ENTITY SearchHighlight2.text          "Firefox now lets you search easily on your favourite sites.">

SearchHighlight1.* and SearchHighlight2.* are the same... I hope this just placeholder text awaiting final copy?
(Assignee)

Comment 6

4 years ago
(In reply to Justin Dolske [:Dolske] from comment #5)
> Comment on attachment 8525707 [details] [diff] [review]
> patch
> 
> Review of attachment 8525707 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hmm, general comment: One thing I don't see here is an adjusted message for
> users who already had Yahoo has their default, or who are in locales where
> the default search provider is _not_ changing. Seems like we should still
> have this point out the shiny new UI, but we should avoid making them think
> something changed when it didn't, and may even want to be quite
> explicit/reassuring to those who will not see a search engine change.

Hoping all your comments will be addressed today. Really the patch here is to get the functionality working, the actual content of the panels is being decided imminently.
(Assignee)

Comment 7

4 years ago
Created attachment 8526373 [details] [diff] [review]
patch rev 2

This implements the new mockups from UX with actual text. The panel shows the first five times the user opens the search popup, if they click through them they never show again.
Attachment #8525707 - Attachment is obsolete: true
Attachment #8526373 - Flags: review?(felipc)
Attachment #8526373 - Flags: review?(dolske)
Comment on attachment 8526373 [details] [diff] [review]
patch rev 2

Review of attachment 8526373 [details] [diff] [review]:
-----------------------------------------------------------------

For beta just move the strings to the xul file to not cause l10n changes
Attachment #8526373 - Flags: review?(felipc) → review+
(Assignee)

Comment 9

4 years ago
Landed on beta with hardcoded strings and one margin tweak from UX: https://hg.mozilla.org/releases/mozilla-beta/rev/98c4c4a55139
(Assignee)

Updated

4 years ago
Attachment #8526373 - Flags: review?(dolske)
Comment on attachment 8526373 [details] [diff] [review]
patch rev 2

Review of attachment 8526373 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/jar.mn
@@ +24,5 @@
>  * skin/classic/browser/browser-lightweightTheme.css
>    skin/classic/browser/click-to-play-warning-stripes.png
>  * skin/classic/browser/content-contextmenu.svg
> +  skin/classic/browser/dots.png                             (../shared/dots.png)
> +  skin/classic/browser/dots@2px.png                         (../shared/dots@2px.png)

Err, the double-sized HiDPI images should have a "@2x" suffix, not "@2px".

Harmless, but could you fix that when landing on nightly/aurora?
status-firefox34: --- → fixed
status-firefox35: --- → affected
status-firefox36: --- → affected
tracking-firefox34: --- → +
tracking-firefox35: --- → +
tracking-firefox36: --- → +
(Assignee)

Comment 11

4 years ago
This was backed out due to test failures so still needs to land on beta
status-firefox34: fixed → affected
(Assignee)

Comment 12

4 years ago
Created attachment 8526954 [details] [diff] [review]
fixup patch

Still running this through try but I think this will solve the problems. We have to keep the panels hidden until they are first shown. This also fixes problems on windows and linux where the button steals focus from the search box. A couple of changes from UX. We don't show when the search bar is in the panel and we don't show if there is no text in the search bar.

There is also a test included. It doesn't work on linux for some reason but we were willing to land without tests so I've just disabled it there for now. I'm also disabling the panels in all test suites by default to make sure we don't conflict with other tests.
Attachment #8526954 - Flags: review?(felipc)
Attachment #8526954 - Flags: review?(felipc) → review+
(Assignee)

Comment 13

4 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/84717f538f7b
status-firefox34: affected → fixed
Blocks: 1103197
(Assignee)

Comment 14

4 years ago
Created attachment 8527057 [details] [diff] [review]
trunk patch

This is the trunk patch, it's based on an older patch from bug 1088660 so might need tweaking when that finally lands. This splits out the strings into a DTD.
Attachment #8527057 - Flags: review+
(Assignee)

Comment 15

4 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/51c693c1ca3f disables the test everywhere as it turns out to be intermittent :(
Flags: qe-verify+
QA Contact: catalin.varga
Depends on: 1105286
Verified as fixed using:

FF 43.RC1
Build Id:20141125180439
OS: Win7 x64, Ubuntu 14.04 x64, Mac Os X 10.9.5

I found an issue on windows that seems to be related with the suggestion box bug 1105286 .
status-firefox34: fixed → verified
No longer depends on: 1105286
https://hg.mozilla.org/mozilla-central/rev/edb694f470b8
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox36: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Has this been tested with much longer strings? Is enough to change the value of browser.search.highlightCount to test it?
I don't think we tested it with much longer strings but the popup should auto-adjust to the strings.
Yeah, changing that pref is all you need to test it. Just set it to any value greater than 0 and start typing in the searchbar.
I managed to translate these strings before today's build, so far so good (my strings' lenght is almost double compared to English).
Depends on: 1105286
Verified as fixed using:

FF 35
Build id: 20141218174327
OS: Mac Os X 10.9.5, Ubuntu 14.04 x64, Win 7 x64
status-firefox35: fixed → verified

Updated

3 years ago
Depends on: 1130480
Verified as  fixed using the following environment:

FF 36.0b9
Build Id: 20150212154903
OS: Win 8.1 x86, Ubuntu 12.04 x86, Mac Os X 10.9.5
status-firefox36: fixed → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.