960725 - Rename Search Settings and reorder Customize list

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

From https://bugzilla.mozilla.org/show_bug.cgi?id=942875#c26 :

Order should be:
Home
Search
Tabs
Import from Android

Comment 1

[Chenxia Liu [:liuche]]

This slightly depends on adding the Home settings in bug 942875.

Comment 2

[Chenxia Liu [:liuche]]

Hi raymond, I wanted to let you know that bug 942875 has landed. Since we'd like to get this into 29, I just wanted to check and see if you think you've got this bug covered - I can mentor and review for you. If not, let me know and I'd be happy to take it!

Comment 3

[raymond [:retornam] (inactive)]

(In reply to Chenxia Liu [:liuche] from comment #2)
> Hi raymond, I wanted to let you know that bug 942875 has landed. Since we'd
> like to get this into 29, I just wanted to check and see if you think you've
> got this bug covered - I can mentor and review for you. If not, let me know
> and I'd be happy to take it!

Hi Chenxia,
I'd like to work on this bug. I'll ping on IRC if I have any questions. Thanks

Comment 4

[Chenxia Liu [:liuche]]

Hi Raymond, we're getting really close to the end of 29 (end of this work week), so just a friendly ping to see if you think you'll get to this in the next day or so. If not, I'm more than happy to take this bug and do it.

Comment 5

[raymond [:retornam] (inactive)]

(In reply to Chenxia Liu [:liuche] from comment #4)
> Hi Raymond, we're getting really close to the end of 29 (end of this work
> week), so just a friendly ping to see if you think you'll get to this in the
> next day or so. If not, I'm more than happy to take this bug and do it.

I was on PTO from Monday to Wednesday. I'm back today and will submit a patch today. Please ping me on IRC when you get in. Sorry for the delay.

Comment 6

[raymond [:retornam] (inactive)]

Attachment: bug-960725.patch

Comment 7

[Chenxia Liu [:liuche]]

Comment on attachment 8368203 [details] [diff] [review]
bug-960725.patch

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

Great job!

Just one thing (localization string), and then I think we're good to go!

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +65,5 @@
>  <!ENTITY settings "Settings">
>  <!ENTITY settings_title "Settings">
>  <!ENTITY pref_category_advanced "Advanced">
>  <!ENTITY pref_category_customize "Customize">
> +<!ENTITY pref_category_search2 "Search">

We'll have to rename this ENTITY, because of the way we do localization.

Basically, the guideline is that if we change a string, we should rename the ENTITY name so that localizers know the string has changed, and that it needs to be re-localized.

Let's just change the entity name to pref_category_search3.

Comment 8

[Chenxia Liu [:liuche]]

Oh, and one last thing, please update the patch message to match this bug (instead of being the bug description for bug 942875), thanks!

Comment 9

[Chenxia Liu [:liuche]]

Attachment: Screenshot: tablet customize settings

This looks great!

Try: https://tbpl.mozilla.org/?tree=Try&rev=2bd50e0a25ba

Comment 10

[raymond [:retornam] (inactive)]

Attachment: bug-960725.patch

Comment 11

[Chenxia Liu [:liuche]]

Comment on attachment 8368619 [details] [diff] [review]
bug-960725.patch

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

Looks great! One last thing regarding the bug commit message: they should match the bug name exactly, and be sure to append me as a reviewer.

So it should look like:
"Bug 960725 - Rename Search Settings and reorder Customize list. r=liuche"

Thanks!

Comment 12

[raymond [:retornam] (inactive)]

Attachment: bug-960725.patch

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/0fec802029d0