964946 - move bookmarks.inc strings into android_strings.dtd

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[:Margaret Leibovic]

I don't think this file is used anymore, we should get rid of it if that's the case.

Comment 1

[Axel Hecht [:Pike]]

And we should move the strings used from bookmarks.inc into android_strings.dtd. That is one of the culprits that caused bug 1084454.

Comment 3

[Allison Naaktgeboren :ally]

Context/conversation on this topic has happened in a couple of places. We'll use this one to track the work.

- https://bugzilla.mozilla.org/show_bug.cgi?id=1036465#c14 (why this helps localizers)

- https://bugzilla.mozilla.org/show_bug.cgi?id=1189790#c20 - comments 20 - 30

Comment 4

[Allison Naaktgeboren :ally]

Attachment: moveBookmarkStringsFromINCtoDtd-part1

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8653782 [details] [diff] [review]
moveBookmarkStringsFromINCtoDtd-part1

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

Do we need to change how these Strings are accessed? It looks like the Strings are pre-processed into strings.xml.in [1] and I feel we should be specifying entities there (e.g. `&bookmarks_title;`).

Also, should we be removing the Strings from bookmarks.inc? I imagine these would conflict and make the scripts angry (or silently drop things).

Flag me for rereview if these don't need to be changed.

[1]: https://mxr.mozilla.org/mozilla-central/search?string=bookmarkdefaults_title_&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

---
Remaining comments are r+ w/ nits.

I assume since we're using the same entity name these Strings won't be retranslated which seems fine because we're not changing them.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +701,5 @@
>  <!ENTITY restriction_disallow_clear_history_title2 "Disable \'Clear browsing history\'">
>  <!ENTITY restriction_disallow_master_password_title2 "Disable master password">
>  <!ENTITY restriction_disallow_guest_browsing_title2 "Disable Guest Browsing">
> +<!-- Default Bookmarks titles-->
> +<!ENTITY bookmarks_title "Mobile">

Transfer the localization notes over for all of these too.

@@ +702,5 @@
>  <!ENTITY restriction_disallow_master_password_title2 "Disable master password">
>  <!ENTITY restriction_disallow_guest_browsing_title2 "Disable Guest Browsing">
> +<!-- Default Bookmarks titles-->
> +<!ENTITY bookmarks_title "Mobile">
> +<!ENTITY bookmarks_about_browser "Firefox: About your browser">

Given my assumption about retranslation, we probably want to keep this entity the same, despite the inconsistent style.

Comment 6

[Allison Naaktgeboren :ally]

(In reply to Michael Comella (:mcomella) from comment #5)
> Comment on attachment 8653782 [details] [diff] [review]
> moveBookmarkStringsFromINCtoDtd-part1
> 
> Review of attachment 8653782 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do we need to change how these Strings are accessed? It looks like the
> Strings are pre-processed into strings.xml.in [1] and I feel we should be
> specifying entities there (e.g. `&bookmarks_title;`).
> 
> Also, should we be removing the Strings from bookmarks.inc? I imagine these
> would conflict and make the scripts angry (or silently drop things).

Removing them made the makefile stuff made, so I left that to the bug 1197054 when I remove the build goop that gets angry. Therefore, this patch's effect is a no-op, it only adds strings. The strings.xml.in changes are in the patch on bug 1197054 It enables me to do the patch in bug 1197054 more cleanly and I'd like to be able to back out the build one painlessly if something goes horribly wrong without risking inconsistent behavior in the product (getting the bookmark strings from the wrong place). 

They are, I understand, going to get re-translated regardless of what I name them. The localization tool is not smart enough to transfer things from the .inc world (sounds like this file is locked down?) to the dtd, so that's why I figured it be worth it to make the aboutBrowser name more consistent.

Comment 7

[Francesco Lodolo [:flod]]

It's OK to change the string IDs and make them consistent.

Translation tools eventually will be able to match the string content and suggest the translation from memory. Folks working manually on files will still have to copy them, and can adapt the IDs.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8653782 [details] [diff] [review]
moveBookmarkStringsFromINCtoDtd-part1

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

re comment 6: I believe you addressed all of my blocking issues so r+ w/ the remaining unaddressed nits. :)

Comment 9

[Allison Naaktgeboren :ally]

rolled into and landed with 1197054.

Comment 10

[Allison Naaktgeboren :ally]

https://bugzilla.mozilla.org/show_bug.cgi?id=1197054#c10