898151 - The new customise search engine page lacks an informational graphic.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Add-on Manager, defect)

Description

[Chris Kitching [:ckitching]]

According to the graphics provided:
https://bug891115.bugzilla.mozilla.org/attachment.cgi?id=772318
There should be a nifty "Tip" graphic illustrating how to add new search providers present on the customise search providers page. There is not - let's fix that.

Comment 1

[Chris Kitching [:ckitching]]

Ian - do you have an appropriate graphic to use for this in the appropriate size(s)?

Comment 2

[Ian Barlow (:ibarlow)]

Here you go Chris! http://cl.ly/0C221W2o1L3T

And the string should read: 

TIP: Add any website to your list of search providers by long-pressing on its search field.

Comment 3

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic

A patch adding the shiny feature. Depends on all three patches from 892113.

Comment 4

[:Margaret Leibovic]

Comment on attachment 782899 [details] [diff] [review]
Add informational graphic

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

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +74,5 @@
>  <!ENTITY pref_category_vendor "&vendorShortName;">
>  <!ENTITY pref_category_datareporting "Data choices">
>  <!ENTITY pref_category_installed_search_engines "Installed search engines">
> +<!ENTITY pref_category_add_search_providers "Add more search providers">
> +<!ENTITY pref_search_tip "TIP: Add any website to your list of search providers by long-pressing on its search field.">

Drive-by: Let's add a localization note here to explain how this string is used. I'm concerned that that "TIP:" bit might confuse localizers.

Comment 5

[Chenxia Liu [:liuche]]

Comment on attachment 782899 [details] [diff] [review]
Add informational graphic

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

Looks good, but definitely add a localization note like mfinkle suggests, that "tip" is synonymous with hint and is only all-capitalized for emphasis.

::: mobile/android/base/resources/xml/preferences_search.xml
@@ +17,5 @@
>                          android:title="@string/pref_category_installed_search_engines"/>
> +
> +    <PreferenceCategory android:title="@string/pref_category_add_search_providers"/>
> +
> +    <Preference android:layout="@layout/preference_search_tip" android:enabled="false" android:selectable="false"/>

Can you break these with newlines?

Comment 6

[Chris Kitching [:ckitching]]

A build for Ian's possible perusal:
https://www.dropbox.com/s/v6t2h710dw9cf5v/protip.apk

Breaking with newlines is inconsistently applied throughout the various resource files. Of course I can oblige, but determining exactly when it is appropriate to do so in general isn't obvious.

Comment 7

[Ian Barlow (:ibarlow)]

This looks great Chris. Let's ship it :)

Comment 8

[Chenxia Liu [:liuche]]

> Breaking with newlines is inconsistently applied throughout the various
> resource files. Of course I can oblige, but determining exactly when it is
> appropriate to do so in general isn't obvious.

Huh, I mostly observe resource files being broken at every attribute. There's also a vague ordering for the attrs, but you'll have to pick sriram's brain for that.

Comment 9

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V2 - Tidy up the XML

Ah, yes, my bad. Restyled those XML tags. Good to go, methinks.

Comment 10

[Chenxia Liu [:liuche]]

Comment on attachment 784671 [details] [diff] [review]
Add informational graphic V2 - Tidy up the XML

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

Two nits, then I think we're good to go, one more bounce should do it!

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +71,5 @@
>  <!ENTITY pref_category_vendor "&vendorShortName;">
>  <!ENTITY pref_category_datareporting "Data choices">
>  <!ENTITY pref_category_installed_search_engines "Installed search engines">
> +<!ENTITY pref_category_add_search_providers "Add more search providers">
> +<!ENTITY pref_search_tip "TIP: Add any website to your list of search providers by long-pressing on its search field.">

Localization note?

::: mobile/android/base/resources/xml/preferences_search.xml
@@ +15,5 @@
>  
>      <org.mozilla.gecko.preferences.SearchPreferenceCategory
>                          android:title="@string/pref_category_installed_search_engines"/>
> +
> +    <PreferenceCategory android:title="@string/pref_category_add_search_providers"/>

I missed this end-tag the first time, but you caught it when we were looking over your code. PrefCategory should enclose the tip preference.

Comment 11

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V3 - Localisation note

Ah, sorry about that. Here's a revised patch - added localisation note and made the XML be correctly nested.

Comment 12

[Chenxia Liu [:liuche]]

Comment on attachment 784727 [details] [diff] [review]
Add informational graphic V3 - Localisation note

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

Looks good. Unbitrot the patch, and then upload a patch for landing on inbound.

Comment 13

[Chris Kitching [:ckitching]]

The bitrot is just because this one was built atop the patches from bug 892113. I'll mark the dependancy and hopefully things will land in the right order... (That one should be landing imminently, too)

Comment 14

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V4 - Fighting bitrot.

Comment 15

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V5 - Fighting bitrot.

Comment 16

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V6 fx-team-ification

Comment 17

[Chenxia Liu [:liuche]]

Landed in fx-team: https://hg.mozilla.org/integration/fx-team/rev/aa3a115f998b

Comment 18

[Tim Taubert [:ttaubert] (inactive)]

Backed out together with bug 892113:

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

Comment 19

[Chris Kitching [:ckitching]]

Attachment: Add informational graphic V7 - Remove string changes.

Comment 20

[Chris Kitching [:ckitching]]

Attachment: String changes only.

As for Bug 892113, seperating out the string changes out so there is still hope of getting this out in 25.

Comment 21

[Chenxia Liu [:liuche]]

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

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c057ecd8f9b4