Closed Bug 374491 Opened 15 years ago Closed 12 years ago

audit the .dtd / .properties files in places and remove unused strings

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3.7a5

People

(Reporter: moco, Assigned: tdowner)

References

Details

Attachments

(2 files, 5 obsolete files)

audit the .dtd / .properties files in places and remove unused strings

spun off from today's l10n meeting.
Blocks: 371926
Duplicate of this bug: 382201
see also mozilla/browser/locales/en-US/profile/bookmarks.html, which gets used on a new profile.
Blocks: 382358
These entities in places.dtd are not used, entities from global/locale/editMenuOverlay.dtd are used instead:

<!ENTITY cmd.select_all.label "Select All">
<!ENTITY cmd.select_all.accesskey "A">
<!ENTITY cmd.select_all.key "a">

<!ENTITY cmd.edit_cut.label         "Cut">
<!ENTITY cmd.edit_cut.accesskey     "t">
<!ENTITY cmd.edit_cut.key           "x">
<!ENTITY cmd.edit_copy.label        "Copy">
<!ENTITY cmd.edit_copy.accesskey    "C">
<!ENTITY cmd.edit_copy.key          "c">
<!ENTITY cmd.edit_paste.label       "Paste">
<!ENTITY cmd.edit_paste.accesskey   "P">
<!ENTITY cmd.edit_paste.key         "v">
<!ENTITY cmd.edit_delete.label      "Delete">
<!ENTITY cmd.edit_delete.accesskey  "D">
<!ENTITY cmd.edit_undo.label        "Undo">
<!ENTITY cmd.edit_undo.accesskey    "U">
<!ENTITY cmd.edit_undo.key          "z">
<!ENTITY cmd.edit_redo.label        "Redo">
<!ENTITY cmd.edit_redo.accesskey    "R">
<!ENTITY cmd.edit_redo.key          "y">
OS: Windows XP → All
Hardware: PC → All
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
Attached patch WIP 1 (obsolete) — Splinter Review
Just covers the places.dtd so far
Attachment #443784 - Flags: feedback?(seth)
Attachment #443784 - Flags: feedback?(seth) → feedback?(mak77)
Comment on attachment 443784 [details] [diff] [review]
WIP 1

Looks good, and i can't see use of those.
Attachment #443784 - Flags: feedback?(mak77) → review+
please do in multiple patches, one per file.
Notice in Bug 563031 KaiRo has done some analysis and you can gather data from there
Attached patch places.dtd patch v1 (obsolete) — Splinter Review
Ok, there were a few lines that I missed, I think this covers them all now.
Assignee: nobody → tyler.downer
Attachment #443784 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #443797 - Flags: review?(mak77)
Attached patch places.properties patch v1 (obsolete) — Splinter Review
This covers places.properties.
Attachment #443801 - Flags: review?(mak77)
Attached patch editBookmarkOverlay.dtd patch v1 (obsolete) — Splinter Review
This covers editBookmarkOverlay.dtd
Attachment #443803 - Flags: review?(mak77)
Comment on attachment 443797 [details] [diff] [review]
places.dtd patch v1

>diff --git a/browser/locales/en-US/chrome/browser/places/places.dtd b/browser/locales/en-US/chrome/browser/places/places.dtd
> <!ENTITY search.label                              "Search:">
> <!ENTITY search.accesskey                          "S">
>-<!ENTITY search.scopeFolder.label                  "Selected Folder">
>-<!ENTITY search.scopeFolder.accesskey              "r">

This is in http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.xul#409

>-<!ENTITY saveSearch.label                          "Save">
>-<!ENTITY saveSearch.accesskey                      "S">

this one in http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.xul#415

>-<!ENTITY detailsPane.selectAnItemText.description "Select an item to view and edit its properties">
>+<!ENTITY detailsPane.selectAnItemText.description "Select an item to view and edit its properties">

What causes this last line change? newline? Or is it just a diff glitch?

>\ No newline at end of file

Please add new line at the end of the file
Attachment #443797 - Flags: review?(mak77) → review-
Comment on attachment 443801 [details] [diff] [review]
places.properties patch v1

>diff --git a/browser/locales/en-US/chrome/browser/places/places.properties b/browser/locales/en-US/chrome/browser/places/places.properties

>-sortByName=Sort '%S' by Name
>-sortByNameGeneric=Sort by Name
>-view.sortBy.name.label=Sort by Name
>-view.sortBy.name.accesskey=N
>-view.sortBy.url.label=Sort by Location
>-view.sortBy.url.accesskey=L
>-view.sortBy.date.label=Sort by Visit Date
>-view.sortBy.date.accesskey=V
>-view.sortBy.visitCount.label=Sort by Visit Count
>-view.sortBy.visitCount.accesskey=C
>-view.sortBy.keyword.label=Sort by Keyword
>-view.sortBy.keyword.accesskey=K
>-view.sortBy.description.label=Sort by Description
>-view.sortBy.description.accesskey=D
>-view.sortBy.dateAdded.label=Sort by Added
>-view.sortBy.dateAdded.accesskey=e
>-view.sortBy.lastModified.label=Sort by Last Modified
>-view.sortBy.lastModified.accesskey=M
>-view.sortBy.tags.label=Sort by Tags
>-view.sortBy.tags.accesskey=T

these are used (name is builkt dynamically) http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.js#1212

> OrganizerQueryHistory=History
>-OrganizerQueryDownloads=Downloads
> OrganizerQueryAllBookmarks=All Bookmarks
> OrganizerQueryTags=Tags

Please leave Downloads in even if we don't use it, i think is being worked on right now

>-# LOCALIZATION NOTE (tagResultLabel) :
>-# This is what we use to form the label (for screen readers)
>-# for url bar autocomplete results of type "tag"
>-# See createResultLabel() in urlbarBindings.xml 
>-tagResultLabel=Tag
>-# LOCALIZATION NOTE (bookmarkResultLabel) :
>-# This is what we use to form the label (for screen readers)
>-# for url bar autocomplete results of type "bookmark"
>-# See createResultLabel() in urlbarBindings.xml 
>-bookmarkResultLabel=Bookmark

These are used http://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#695

>-lockPromptInfoButton.accessKey=L
>+lockPromptInfoButton.accessKey=L
>\ No newline at end of file

Same as previous patch
Attachment #443801 - Flags: review?(mak77) → review-
Attachment #443803 - Flags: review?(mak77) → review-
Comment on attachment 443803 [details] [diff] [review]
editBookmarkOverlay.dtd patch v1

>diff --git a/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd b/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd
>--- a/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd
>+++ b/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd
>@@ -1,10 +1,8 @@
>-<!ENTITY editBookmarkOverlay.name.label                      "Name:">
>-<!ENTITY editBookmarkOverlay.name.accesskey                  "N">

used http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/editBookmarkOverlay.xul#63

>-<!ENTITY editBookmarkOverlay.newFolderButton.accesskey       "o">
>+<!ENTITY editBookmarkOverlay.newFolderButton.accesskey       "o">
>\ No newline at end of file

same as previous patches
I think the last change is a whitespace thing. I can take that out. I'll put those strings back in and get us going here ;)
Thank you, I really appreciate your effort here!
Attached patch places.dtd patch v2 (obsolete) — Splinter Review
Ok, I think this address all the comments on places.dtd
Attachment #443797 - Attachment is obsolete: true
Attachment #443961 - Flags: review?(mak77)
Comment on attachment 443803 [details] [diff] [review]
editBookmarkOverlay.dtd patch v1

nothing needs to be taken from here then :)
Attachment #443803 - Attachment is obsolete: true
(In reply to comment #18)
> (From update of attachment 443803 [details] [diff] [review])
> nothing needs to be taken from here then :)

you could add the missing newline at the end of the file though!
Looking in there, I took that newline out when I created the patch, so, there is a newline already ;) I didn't realize it at first. Sorry for the confusion
Here is places.properties :)
Attachment #443801 - Attachment is obsolete: true
Attachment #443971 - Flags: review?(mak77)
Comment on attachment 443961 [details] [diff] [review]
places.dtd patch v2

>diff --git a/browser/locales/en-US/chrome/browser/places/places.dtd b/browser/locales/en-US/chrome/browser/places/places.dtd

>-<!ENTITY view.unsorted.label            "Unsorted">
>-<!ENTITY view.unsorted.accesskey        "U">

argh dunno how i missed this before, but it's used, sorry my fault :(
http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.xul#304

Will be perfect once this is fixed
Attachment #443961 - Flags: review?(mak77) → review-
Comment on attachment 443971 [details] [diff] [review]
places.properties patch v2

Looks perfect! thanks!
Attachment #443971 - Flags: review?(mak77) → review+
Duplicate of this bug: 563031
Ok, there it is I think :)
Attachment #443961 - Attachment is obsolete: true
Attachment #443990 - Flags: review?(mak77)
Attachment #443990 - Flags: review?(mak77) → review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/40dc6e65a824
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a5
Status: RESOLVED → VERIFIED
Comment on attachment 443971 [details] [diff] [review]
places.properties patch v2

> # LOCALIZATION NOTE (bookmarksBackupFilename) :
> # %S will be replaced by the current date in ISO 8601 format, YYYY-MM-DD.
> # The resulting string will be suggested as a filename, so make sure that you're
> # only using characters legal for file names. Consider falling back to the
> # en-US value if you have to use non-ascii characters.
>-bookmarksBackupFilename=Bookmarks %S.html
>-bookmarksBackupFilenameJSON=Bookmarks %S.json

The localization note should be removed as well...
(In reply to comment #27)
> (From update of attachment 443971 [details] [diff] [review])
> > # LOCALIZATION NOTE (bookmarksBackupFilename) :

> The localization note should be removed as well...

Right, we need a followup push
Pushed removal of the no more useful note:
http://hg.mozilla.org/mozilla-central/rev/549f77777959

Thanks for the notice.
You need to log in before you can comment on or make changes to this bug.