Bug 114150 - Remove duplication of strings found in linux and windows file dialogs
Summary: Remove duplication of strings found in linux and windows file dialogs
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0
Keywords:
Depends on:
Blocks: File-Dialog l10n-Optimization
  Show dependency treegraph
 
Reported: 2017-11-29 18:25 UTC by Yousuf Philips (jay) (retired)
Modified: 2022-03-11 12:32 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yousuf Philips (jay) (retired) 2017-11-29 18:25:25 UTC
The file dialogs in linux and windows arent sharing the same strings, resulting in the strings being translated twice.

For example, STR_FPICKER_AUTO_EXTENSION and STR_SVT_FILEPICKER_AUTO_EXTENSION are both assigned to "~Automatic file name extension".

https://opengrok.libreoffice.org/xref/core/vcl/inc/strings.hrc#93

https://opengrok.libreoffice.org/xref/core/include/fpicker/strings.hrc#14
Comment 1 Julien Nabet 2022-02-27 21:27:54 UTC
Caolán: Taking a look, I see for example:
julien@debianamd:~/lo/libreoffice$ git grep -n STR_SVT_FILEPICKER_AUTO_EXTENSION
fpicker/source/aqua/resourceprovider.mm:51:    { CHECKBOX_AUTOEXTENSION,                   STR_SVT_FILEPICKER_AUTO_EXTENSION },
fpicker/source/office/iodlg.cxx:2070:        m_xImpl->m_xCbAutoExtension->set_label( FpsResId( STR_SVT_FILEPICKER_AUTO_EXTENSION ) );
fpicker/source/win32/VistaFilePickerImpl.cxx:643:        iCustom->AddCheckButton (nControlId, o3tl::toW(FpsResId(STR_SVT_FILEPICKER_AUTO_EXTENSION).replaceFirst("~","").getStr()), true);
fpicker/source/win32/resourceprovider.cxx:55:    { CHECKBOX_AUTOEXTENSION,                   STR_SVT_FILEPICKER_AUTO_EXTENSION },
include/fpicker/strings.hrc:14:#define STR_SVT_FILEPICKER_AUTO_EXTENSION           NC_("STR_SVT_FILEPICKER_AUTO_EXTENSION", "~Automatic file name extension")
julien@debianamd:~/lo/libreoffice$ git grep -n STR_FPICKER_AUTO_EXTENSION
vcl/inc/strings.hrc:70:#define STR_FPICKER_AUTO_EXTENSION                   NC_("STR_FPICKER_AUTO_EXTENSION", "~Automatic file name extension")
vcl/qt5/QtFilePicker.cxx:637:            resId = STR_FPICKER_AUTO_EXTENSION;
vcl/unx/gtk3/fpicker/resourceprovider.cxx:37:    { CHECKBOX_AUTOEXTENSION,                   STR_FPICKER_AUTO_EXTENSION },
vcl/unx/gtk3_kde5/gtk3_kde5_filepicker.cxx:224:            resId = STR_FPICKER_AUTO_EXTENSION;

but there are other strings.
I thought about removing those in vcl/inc/strings.hrc and use those from 
include/fpicker/strings.hrc

What do you think? Unless fpicker dir should be removed at the end? I must recognize I don't understand why there's specific fpicker part whereas there's fpicker part too in vcl and in this last part we also see native file pickers.
Comment 2 Caolán McNamara 2022-03-07 11:19:04 UTC
so, fwiw this duplication happened in 2011

commit aeffd7f25f6bf664ee5536942bd48407447867f7
Date:   Fri Nov 4 21:49:28 2011 +0000

    move required resources down from fpicker & svtools into vcl

once upon a time all the file pickers in vcl were in fpicker but the gtk one got moved into vcl while the "generic", aqua and windows file pickers stayed in fpicker.

It is certainly possible now with the .hrc and gettext approach to use the fpicker translations from vcl. But it so, then its important to go via the fps.mo instead of the vcl.mo in that case, e.g.

adding a OUString FpsResId(TranslateId aId) { return Translate::get(aId, Translate::Create("fps")); };

to the appropriate place in vcl (or inline in some new include/fpicker/something.hxx) and change the uses of VclResId for those strings to that FpsResId
Comment 3 Julien Nabet 2022-03-07 18:18:57 UTC
Thank you Caolán for your feedback.
After having read what you indicated, I did a make "git grep -n 'TranslateId aId'" and found:
...
fpicker/inc/fpsofficeResMgr.hxx:13:inline OUString FpsResId(TranslateId aId) { return Translate::get(aId, Translate::Create("fps")); };
...
include/editeng/eerdll.hxx:34:OUString EDITENG_DLLPUBLIC EditResId(TranslateId aId);
include/sfx2/sfxresid.hxx:26:SFX2_DLLPUBLIC OUString SfxResId(TranslateId aId);
include/svtools/svtresid.hxx:28:SVT_DLLPUBLIC OUString SvtResId(TranslateId aId);
include/svx/dialmgr.hxx:27:SVXCORE_DLLPUBLIC OUString SvxResId(TranslateId aId);
...
vcl/source/app/svdata.cxx:259:OUString VclResId(TranslateId aId)
...

so it could be a 3rd steps plan:
1)
- move "fpsofficeResMgr.hxx" from "fpicker/inc" to "include/fpicker"
- keep strings in include/fpicker/
- change required files so build is ok
build to check it's ok

2)
- modify vcl files which have dups so it uses FpsResId
build to check it's ok

3)
- remove duplicate strings from vcl
build to check it's ok


As an alternative, but which would perhaps require more work, would it be relevant to merge fpicker part in vcl?
After all, if gtk part is in vcl, why not the other fpickers?
Comment 4 Caolán McNamara 2022-03-07 19:44:38 UTC
> so it could be a 3rd steps plan:
> 1)
> - move "fpsofficeResMgr.hxx" from "fpicker/inc" to "include/fpicker"
> - keep strings in include/fpicker/
> - change required files so build is ok
> build to check it's ok
> 
> 2)
> - modify vcl files which have dups so it uses FpsResId
> build to check it's ok
> 
> 3)
> - remove duplicate strings from vcl
> build to check it's ok

Yup. Also, testing with a non-English UI (because the english strings are in the source and don't get pulled from the .mo) should show that the translations are pulled from the right place.
 
> As an alternative, but which would perhaps require more work, would it be
> relevant to merge fpicker part in vcl?
> After all, if gtk part is in vcl, why not the other fpickers?

Yeah, it probably also makes sense to move the fpicker into vcl, but it is more work and platform specific.
Comment 5 Commit Notification 2022-03-11 12:11:12 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3307afa99c28fd282094e9558795b0a20ba5c6a0

tdf#114150: Remove duplication of strings found in file dialogs

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Julien Nabet 2022-03-11 12:32:31 UTC
Since it can only be applied on master branch because of string freeze on 7.3 branch, let's put this one to FIXED.