Closed
Bug 964946
Opened 11 years ago
Closed 10 years ago
move bookmarks.inc strings into android_strings.dtd
Categories
(Firefox for Android Graveyard :: Data Providers, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Margaret, Assigned: ally)
References
Details
Attachments
(1 file)
1.61 KB,
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
I don't think this file is used anymore, we should get rid of it if that's the case.
![]() |
||
Comment 1•11 years ago
|
||
And we should move the strings used from bookmarks.inc into android_strings.dtd. That is one of the culprits that caused bug 1084454.
Assignee | ||
Updated•10 years ago
|
Summary: bookmarks.json.in looks unused → bookmarks.json.in looks unused, move bookmarks.inc strings into android_strings.dtd
Assignee | ||
Comment 3•10 years ago
|
||
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
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ally
Assignee | ||
Updated•10 years ago
|
Summary: bookmarks.json.in looks unused, move bookmarks.inc strings into android_strings.dtd → move bookmarks.inc strings into android_strings.dtd
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8653782 -
Flags: review?(michael.l.comella)
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.
Attachment #8653782 -
Flags: review?(michael.l.comella) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
(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•10 years ago
|
||
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 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. :)
Attachment #8653782 -
Flags: feedback+ → review+
Assignee | ||
Comment 9•10 years ago
|
||
rolled into and landed with 1197054.
Assignee | ||
Comment 10•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•