1193638 - When a filepicker is displayed the Edit menu gets delocalized

Status: REOPENED

(Core :: Widget: Cocoa, defect, P3)

Description

[Thomasy]

Attachment: zh-TW case

Open the file dialog/picker on OSX 10.10.4. The menu bar change to Edit and it is not localized.

Comment 1

[Thomasy]

Attachment: Chrome

While Chrome will not change menu bar.

Comment 2

[Thomasy]

Attachment: ja-JP case

For localization jp-JP, and pl are tested and have the same bug.

Comment 3

[Thomasy]

Attachment: pl case

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Please check for a regression range among mozilla-central nightlies.  In other words, please check to find out which mozilla-central nightly this problem started happening in.

It's possible that the patch for bug 1089363 is the trigger here.

Comment 5

[Thomasy]

I am sure that the problem is not bug 1089363. Since I created the patch for Thunderbird (bug 1185268) and Instantbird (bug 1192629), I found the bug is there before I check-in the ones for them. I will make some regression.

Comment 6

[Thomasy]

I think it may duplicate bug 475290.

Comment 7

[Thomasy]

I new version can be found here

https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsMenuUtilsX.mm?offset=200#114

Comment 9

[Steven Michaud [:smichaud] (Retired)]

This bug is old enough not to count as a "regression".

I'm very unlikely to have time to work on this in the 45 or so days before I retire.  So someone else will have to pick this up.

Comment 10

[Thomasy]

Attachment: bug1193638_rev1.patch

Comment 11

[Thomasy]

Attachment: bug1193638_rev1.patch

Comment 12

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8651460 [details] [diff] [review]
bug1193638_rev1.patch

One question and an observation:

1) Does this actually work?  For example, if you do a zh-TW build with this patch, is the Edit menu properly localized that you get when you open a filepicker?

2) If you're going to localize the menu item labels, you should also localize the accelerator keys (cmd-key options).  nsMenuBarX::GetLocalizedAccelKey() may provide some clues how to do this.

Comment 13

[Thomasy]

Attachment: zh-TW_menu.png

Comment 14

[Thomasy]

Attachment: regular_zh-TW_edit_menu.png

Comment 15

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #12)
> Comment on attachment 8651460 [details] [diff] [review]
> bug1193638_rev1.patch
> 
> One question and an observation:
> 
> 1) Does this actually work?  For example, if you do a zh-TW build with this
> patch, is the Edit menu properly localized that you get when you open a
> filepicker?

This is actually work as in attachment 8652004 [details]. And I have also verify is try build (https://treeherder.mozilla.org/#/jobs?repo=try&revision=62299afc8f8a)

> 
> 2) If you're going to localize the menu item labels, you should also
> localize the accelerator keys (cmd-key options). 
> nsMenuBarX::GetLocalizedAccelKey() may provide some clues how to do this.

The attachment 8652007 [details] shows the regular edit menu. The proposed patch work since the accelerator seems keep in English. Do you mean it should like https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties?offset=100#137

Comment 16

[Steven Michaud [:smichaud] (Retired)]

> And I have also verify is try build
> (https://treeherder.mozilla.org/#/jobs?repo=try&revision=62299afc8f8a)

That try build isn't localized -- it's en-US only.

Do zh-TW build with your patch and see if that also works.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

> The proposed patch work since the accelerator seems keep in English.

Then I really don't know what you're trying to accomplish with your patch :-(

Comment 18

[Steven Michaud [:smichaud] (Retired)]

> The proposed patch work since the accelerator seems keep in English.

I may have misunderstood what this is supposed to mean.  Do you mean to say that the accelerators are the same in zh-TW as they are in en-US?  Even if that's true, it's not good enough.  For you to be able to get away with not localizing the accelerators, they'd need to be the same in *all* locales.  Are they?  You need to find out.

Comment 19

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #16)
> > And I have also verify is try build
> > (https://treeherder.mozilla.org/#/jobs?repo=try&revision=62299afc8f8a)
> 
> That try build isn't localized -- it's en-US only.
> 
> Do zh-TW build with your patch and see if that also works.

I do build my own zh-TW with my patch. The try build is used to verify if the default en-US locale is as before.

Comment 20

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #18)
> > The proposed patch work since the accelerator seems keep in English.
> 
> I may have misunderstood what this is supposed to mean.  Do you mean to say
> that the accelerators are the same in zh-TW as they are in en-US?  Even if
> that's true, it's not good enough.  For you to be able to get away with not
> localizing the accelerators, they'd need to be the same in *all* locales. 
> Are they?  You need to find out.

Yes I mean that the both locale have the same accelerator. While I have checked for example "Select All" the accelerator is indeed different from language to language.

./cy/browser/chrome/browser/browser.dtd:<!ENTITY selectAllCmd.accesskey       "P">
./da/browser/chrome/browser/browser.dtd:<!ENTITY selectAllCmd.accesskey       "a">
./zh-TW/browser/chrome/browser/browser.dtd:<!ENTITY selectAllCmd.accesskey       "A">

I suggested that we will use the same approach in https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsMenuBarX.mm#167 with for example "quitMenuitem.label" for the label and "quitMenuitem.key" for the accelerators. I will work on localizing the accelerators.

Comment 21

[Thomasy]

Attachment: Result of the proposed patch

Comment 22

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #17)
> > The proposed patch work since the accelerator seems keep in English.
> 
> Then I really don't know what you're trying to accomplish with your patch :-(

I compare the filepicker before and after applying the new patch as in attachment 8652128 [details]. Localizing the edit menu is the main purpose of the patch. While the edit menu which is hard-coded before applying the patch.

But one weird thing which also in attachment 358795 [details] of bug 475290. The menu item "Undo" and "Redo" which will be localized even before the patch applied.

Comment 23

[Thomasy]

Attachment: bug1193638_rev2.patch

The revised version with accelerators.

Comment 24

[Thomasy]

Attachment: patch for zh-TW locale

The zh-TW locale (http://hg.mozilla.org/l10n-central/zh-TW/) for the patch.

Comment 25

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #24)

So your patch will need to provide a localization of editMenuItem.properties for every supported locale! :-(  This is what I was afraid of when I asked (in comment #12) "does this actually work"?  Clearly your original patch doesn't -- not as it stands.

I'm afraid I'm going to have to give up on this bug.  It's now about 30 days until I retire, and I simply don't have time for the work that will be required here.  I have *lots* of other things I need to finish, all of them more urgent than this bug.

I'm also not a build system or localization expert.

Comment 26

[Thomasy]

Markus, any suggestions?

Comment 27

[Stefan [:stefanh]]

JFTR I think the normal process when providing the ability to localize is that you first add the en-US localization and then localizers themself will add the new strings for their respective languages. Localizers have a dashboard that notifies them of added strings.

Comment 28

[Steven Michaud [:smichaud] (Retired)]

Another question that needs to be answered:

Do we really need editMenuItem.properties (and all its localizations)?  Or is the information it contains already available elsewhere (and usable from that location)?

Comment 29

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #28)
> Another question that needs to be answered:
> 
> Do we really need editMenuItem.properties (and all its localizations)?  Or
> is the information it contains already available elsewhere (and usable from
> that location)?

Just answer part of the question. I summarize the 8 words("edit", "undo", "redo", "cut", "copy", "paste", "delete", "select all") that can be found in current en-US locale. The three files ./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd, ./webapprt/locales/en-US/webapprt/webapp.dtd, ./browser/locales/en-US/chrome/browser/browser.dtd have all the 8 words we need.


Edit:
./toolkit/locales/en-US/chrome/global/devtools/gclicommands.properties:cookieListOutEdit=Edit
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY editMenu.label            "Edit">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY editMenu.label                     "Edit">
./browser/locales/en-US/chrome/browser/devtools/scratchpad.dtd:<!ENTITY editMenu.label               "Edit">
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY editMenu.label         "Edit"> 
./mobile/android/base/locales/en-US/android_strings.dtd:<!ENTITY contextmenu_edit_bookmark "Edit">
./mobile/android/base/locales/en-US/android_strings.dtd:<!ENTITY contextmenu_top_sites_edit "Edit">

Undo:
./toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd:<!ENTITY addon.undoAction.label               "Undo">
./toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd:<!ENTITY addon.undoRemove.label               "Undo">
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY undoCmd.label             "Undo">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY undoCmd.label "Undo">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY undoCmd.label                      "Undo">
./browser/locales/en-US/chrome/browser/browser.properties:lwthemePostInstallNotification.undoButton=Undo
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY undoCmd.label            "Undo">  
./mobile/android/locales/en-US/chrome/sync.properties:notificationDisconnect.button=Undos
./mobile/android/locales/en-US/chrome/aboutAddons.dtd:<!ENTITY addonAction.undo                       "Undo">

Redo:
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY redoCmd.label             "Redo">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY redoCmd.label                      "Redo">
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY redoCmd.label            "Redo">  

Cut:
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY cutCmd.label              "Cut">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY cutCmd.label "Cut">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY cutCmd.label                       "Cut">
./browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties:cut-button.label = Cut
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY cutCmd.label           "Cut">  
./mobile/android/locales/en-US/chrome/browser.properties:contextmenu.cut=Cut

Copy:
./toolkit/locales/en-US/chrome/mozapps/help/help.dtd:<!ENTITY copyCmd.label                  "Copy">
./toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties:styleinspector.contextmenu.copy=Copy
./toolkit/locales/en-US/chrome/global/config.dtd:<!ENTITY copyPref.label "Copy">
./toolkit/locales/en-US/chrome/global/aboutTelemetry.properties:histogramCopy = Copy
./toolkit/locales/en-US/chrome/global/console.dtd:<!ENTITY copyCmd.label       "Copy">  
./toolkit/locales/en-US/chrome/global/commonDialog.dtd:<!ENTITY copyCmd.label    "Copy">
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY copyCmd.label             "Copy">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY copyCmd.label "Copy">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY copyCmd.label                      "Copy">
./browser/locales/en-US/chrome/browser/devtools/jsonview.properties:jsonViewer.Copy=Copy
./browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties:copy-button.label = Copy
./browser/locales/en-US/chrome/browser/pageInfo.dtd:<!ENTITY  copy.label            "Copy">
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY copyCmd.label            "Copy">  
./mobile/android/base/locales/en-US/android_strings.dtd:<!ENTITY button_copy "Copy">
./mobile/android/locales/en-US/chrome/aboutLogins.properties:loginsDialog.copy=Copy
./mobile/android/locales/en-US/chrome/browser.properties:contextmenu.copy=Copy

Paste:
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY pasteCmd.label            "Paste">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY pasteCmd.label "Paste">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY pasteCmd.label                     "Paste">
./browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties:paste-button.label = Paste
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY pasteCmd.label           "Paste">  
./mobile/android/base/locales/en-US/android_strings.dtd:<!ENTITY contextmenu_paste "Paste">
./mobile/android/locales/en-US/chrome/browser.properties:contextmenu.paste=Paste

Delete:
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY deleteCmd.label           "Delete">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY deleteCmd.label "Delete">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY deleteCmd.label                    "Delete">
./browser/locales/en-US/chrome/browser/devtools/projecteditor.properties:projecteditor.deleteLabel=Delete
./browser/locales/en-US/chrome/browser/devtools/projecteditor.properties:projecteditor.deletePromptTitle=Delete
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY deleteCmd.label          "Delete">  
./mobile/android/locales/en-US/chrome/aboutLogins.properties:loginsMenu.delete=Delete
./mobile/android/locales/en-US/chrome/aboutDownloads.dtd:<!ENTITY aboutDownloads.remove                     "Delete">

Select All:
./toolkit/locales/en-US/chrome/mozapps/help/help.dtd:<!ENTITY selectAllCmd.label             "Select All">
./toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties:styleinspector.contextmenu.selectAll=Select All
./toolkit/locales/en-US/chrome/global/commonDialog.dtd:<!ENTITY selectAllCmd.label "Select All">
./toolkit/locales/en-US/chrome/global/editMenuOverlay.dtd:<!ENTITY selectAllCmd.label        "Select All">
./toolkit/locales/en-US/chrome/global/textcontext.dtd:<!ENTITY selectAllCmd.label "Select All">
./webapprt/locales/en-US/webapprt/webapp.dtd:<!ENTITY selectAllCmd.label                 "Select All">
./browser/locales/en-US/chrome/browser/pageInfo.dtd:<!ENTITY  selectall.label       "Select All">
./browser/locales/en-US/chrome/browser/browser.dtd:<!ENTITY selectAllCmd.label         "Select All">  
./mobile/android/locales/en-US/chrome/browser.properties:contextmenu.selectAll=Select All

Comment 30

[Thomasy]

(In reply to Steven Michaud [:smichaud] from comment #28)
> Another question that needs to be answered:
> 
> Do we really need editMenuItem.properties (and all its localizations)?  Or
> is the information it contains already available elsewhere (and usable from
> that location)?

For the ones we already get is in .dtd file which is used for .xul and which may not be our case. Instead the property files are used. Any ideas?

References:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Tutorial/Localization
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Tutorial/Property_Files

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 32

[Stephen A Pohl [:spohl]]

I believe this is no longer occurring, but please reopen if it does.

Comment 33

[Stefan [:stefanh]]

Attachment: non-localized-menu.png

Comment 34

[Stefan [:stefanh]]

Reopening. Using the sv-SE version and MacOS 10.12.6, I see this with the latest nightly and with 99.0.1. See attached screenshot for how the menu looks like when doing an "Open file...".

Comment 35

[Steven Michaud [:smichaud] (Retired)]

I can reproduce this bug on macOS 10.12.6 and 12.3.1. I tested with the FF 99.0.1 zh_TW localized build (Chinese Traditional), with the Preferred Language set to either English (en_US) or traditional Chinese (zh_TW).

Unlike other macOS programs, Firefox rewrites the main menu when Open File or Save Page As is chosen -- all submenus disappear but Firefox and File. I'd bet that's what's behind this bug.

And no, I'm not interested in resuming my work here :-)