TL;DR: When retrieving the file content with localization scripts,
normalize the line ending to LF.
Issue:
When a grdp file containing multi-line strings has CRLF line ending,
the parser doesn't work because the IDS hash is calculated based on the
content of the <message> tag, which expects LF line ending. The multi-
line string ends up having a different expected hash, and the parser
complains about it.
Repro:
Change line ending of front_end/coverage/coverage_strings.grdp to CRLF.
Fix:
Normalize line ending to LF when retrieving the file content.
Change-Id: If3a33978724a4bf9635738c67ada211e5e996e30
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1895994
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
This patch adds the functionality to autofix grdp/grd file paths that are
out of date.
* <part> file entries that reference non-existent grdp files in
devtools_ui_strings.grd are removed.
* Grdp files with the wrong names are renamed (format should be
<folder_name>_strings.grdp) along with the corresponding part file entries
in devtools_ui_strings.grd.
* If more than one grdp files are under a directory, an error is issued to
ask the user to consolidate these grdp files.
This patch will eliminate the need to manually fix grdp/grd inconsistensies
(e.g. https://crrev.com/c/1637591) after unexpected changes (e.g. audits2
renamed to audits: https://crrev.com/c/1614691).
Bug: 941561
Change-Id: Ie413fe070a2f5b4319a12ce07653ec1a79912215
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1703044
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#698917}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 3329fdbaecd905bf1a98a5ae24893d700571ba70
The tags in Module.json are not localized. This change localize them,
so when users type the word in command menu with languages other than
English, the results can show up correctly.
Two scripts edited (check_localized_strings.js, CommandMenu.js).
Grdp changes are generated automatically, with manually added descriptions.
Bug: 941561
Change-Id: I3dc1f8f8b72e21748f5fa84c1c60b9597aa0414a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809673
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Mandy Chen <mandy.chen@microsoft.com>
Commit-Queue: Christy Chen <chrche@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#698223}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: b75a4b07ffd26a61fef0344123701241bb75002c
Currently, if an unsupported JS feature is used, localization presubmit
checks will fail with the raw error message from esprima, the JS parsing
library. This CL wraps a try-catch block around the parsing call and outputs
a nice error message that specifies the file location and the reason.
Before:
Error: Line 9: Unexpected token =
After:
Error: DevTools localization parser failed:
third_party\blink\renderer\devtools\front_end\security\SecurityPanel.js: Line 9: Unexpected token =
This error is likely due to unsupported JavaScript features. Such features
are not supported by eslint either and will cause presubmit to fail. Please
update the code and use official JavaScript features.
Bug: 941561
Change-Id: If945563f84224232155a7de7cf597c5395a0cbb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729729
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#697675}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 2b4a1abab0661e491ab275768f8f174cf5686c46
Since the current localization system uses a unique ID for each string that
is calculated based on the the md5 hash of the string, each string
in the grdp files needs to be unique. However, there are strings in the
frontend that appear multiple times. When such strings are moved across
folders, sometimes a seemingly random grdp file is changed by the autofix
script. (See https://crrev.com/c/1683826 for an example)
This CL introduces shared_strings.grdp, a file dedicated for strings that
are shared across folders/grdp files. Instead of putting these strings in
the grdp files that come first when sorted alphabetically, they live in
shared_strings.grdp and have common descriptions among all instances of
such strings. This way if shared strings need to be moved, it's more clear
what's going on.
Note that this CL contains a lot of changes that are generated automatically
(e.g. shared strings are removed from their current locations), so here's
the list of actual changes that need to be reviewed:
* shared_strings.grdp file is added to front_end/langpacks and front_end/langpacks/
devtools_ui_strings.grd
* path to shared_strings.grdp is added to scripts/localization_utils/localization_utils.js
* in scripts/localization_utils/check_localized_strings.js, the parser marks
strings that appear more than once as shared and set the target grdp file to be
shared_strings.grdp
* shared_strings.grdp file is checked by scripts/check_localizability.js for
localizability violations
* shared_strings.grdp messages have common descriptions added
Bug: 941561
Change-Id: I00db23854656509f2f03988e70adc0109b6e09d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1800918
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#697667}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: d10bf7c5a97cf9184866c1117935c199bacf7ff3
This patch adds a localizability check to the presubmit script that reports
errors for empty descriptions, after https://crrev.com/c/1688716 adds auto-
generated descriptions.
Example error messages:
third_party/blink/renderer/devtools/front_end/accessibility/accessibility_strings.grdp
Line 36: missing description for message with the name "IDS_DEVTOOLS_185551542d4a950d6ed4a90e0875dfde"
third_party/blink/renderer/devtools/front_end/animation/animation_strings.grdp Line 10:
missing <ex> in <ph> tag with the name "BUTTON_TEXTCONTENT"
This patch also improves error reporting by changing full file path to partial
path from src/ so that it's more consistent with the presubmit error style.
Note: there may be new strings added without a description and/or placeholder
example while this patch is on review. In that case I will fix them so the
check can be enabled without any problem.
Bug: 941561
Change-Id: If609d234b67acb76dfd9c66e2f8880d9875034cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693028
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#681975}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1404bfcfd225f2963874e37299d890b1f3119e72
Due to the merge timing of patches, sometimes duplicate grdp messages
can be added and co-exist*. This patch updates the presubmit script to
automatically delete duplicate grdp messages.
- Keep the winning grdp message (i.e. the one that corresponds to the
frontend message that comes first based on its file name when sorted).
- If none of these duplicate messages should be kept, delete them all.
- If none of these duplicate messages should be kept AND the same message
needs to be created in a different grdp file, preserve the longer
description.
In addition, this patch removes unnecessary arguments such as isDebug and
file-level dictionary structures.
*: https://crrev.com/c/1580199 and https://crrev.com/c/1633989 both added
a grdp message for "Image from %s".
Bug: 941561
Change-Id: Ic37f1fc410361eabd0ba710ece73a76d78eb8d34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1656070
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#677799}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: a0dbe54c77da415cfc79ee99bc0cd9e590ffb6ff
The ids key (i.e. name value of a <message> tag) of a grdp message is
IDS_DEVTOOLS_<md5 hash of the string>. Currently if you manually modify
the content of a message and modify the corresponding frontend string,
the ids key can be outdated.*
This patch improves the presubmit script to autofix such issues. Actual
fix of the current problem is in another patch:
https://crrev.com/c/1670315.
*: example: https://crrev.com/c/1637046
Bug: 941561
Change-Id: I2f2b044955aa1033f07a054ac2926392d6ff1687
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661120
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Reviewed-by: Erik Luo <luoe@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#676641}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 36f218b51c5cbc5e1c43e7c42dcf4bd83b50bfdc
Right now if a localizable string is moved to a different folder, the
autofix tool doesn't move the corresponding grdp message to the new grdp
file. I updated the tool to track which grdp file a localizable string is
supposed to be in, and compare it to the actual grdp file when autofixing
issues. When a string is copied to another folder, the description is
automatically copied over to the new grdp file.
Existing issues are fixed up.
Bug: 941561
Change-Id: Iaf4537a8c51b8947b7edb5d54c15dbf8571060d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637590
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#668185}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 6f01dec7da56367f1cf058a6e5b554c1a540846d
This reverts commit 2f05fdbf2ab3458b4b3a3f2a86f908a3a7d04f79.
Reason for revert: Breaks chrome translation, crbug 966547
Original change's description:
> DevTools: Added GRD/GRDP files for all localizable strings in the DevTools.
>
> This change includes...
> * A main GRD file for the DevTools (front_end/langpacks/devtools_ui_strings.grd)
> * GRDP files organized by subfolder
> * A node script that keeps the GRD/GRDP files in sync with the keys in the DevTools frontend
> * A git cl presubmit --upload check that runs the node script
>
> Note: Subsequent changes will add a build step to generate a .pak file that contains these DevTools strings. You can read about the overall approach here (https://bugs.chromium.org/p/chromium/issues/detail?id=941561).
>
>
> Details of this design:
> ======================
> We followed a similar pattern used by WebUI where strings are encoded in GRIT GRD/P files, which are used by a localization service to perform translations. They are also consumed in the build step to generate a .pak file, which is loaded by the browser's resource system.
>
> Chromium documentation:
> * https://www.chromium.org/developers/tools-we-use-in-chromium/grit/grit-users-guide
> * https://www.chromium.org/developers/design-documents/ui-localization
>
> Frontend strings:
> -----------------
> These are the localizable strings that are displayed to the user.
>
> GRDP <message> strings:
> -----------------------
> Each frontend string has a corresponding <message> entry in a GRDP file. These entries are what the localization service will use to perform localizations. It's also the input to the GRIT compiler, which generates a .pak file, which is loaded by the browser's resource_bundle system.
>
> GRDP <messsage> placeholders:
> ----------------------------
> Frontend strings use placeholders, which are used to substitute in values at runtime.
> For example,
> 'This string has %s two placeholder %.2f.'
>
> Since the order of the string may change in a different language, we need to encode the order of the placeholders. As such, in the GRDP file you'll find %s replaced with $[1-9].
>
> For example,
> 'This string has <ph name="ph1">$1s</ph> two placeholder <ph name="ph2">$2.2f</ph>.'
>
> Also, note that the precision and type of the placeholder is maintained (i.e. .2f).
>
> Detecting changes:
> -----------------
> The check_localizable_resources.js script performs the following check and generates an error if there are any changes that need to be made to a GRDP file.
>
> 1. Parses the frontend strings and hashes them.
> 2. Reads the messages from the GRDP files and hashes them.
> 3. Uses a difference between these two sets to report which strings need to be added and/or removed from GRDP files.
>
> Optionally, the user can specify --autofix and it will automatically update the appropriate GRDP files.
>
> Presubmit check:
> ---------------
> Running git cl presubmit --upload will run the check_localizable_resources.js script with the --autofix argument.
>
> If there are any changes, they reported to the user like this.
>
> ** Presubmit ERRORS **
> Error: Found changes to localizable DevTools strings.
> DevTools localizable resources checker has updated the appropriate grdp file(s).
> Manually write a description for any new <message> entries.
> Use git status to see what has changed.
>
> BUG=941561
>
> Change-Id: I5ac1656a037a6aaffeb4f64b103c4daec28be39a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613921
> Reviewed-by: Joel Einbinder <einbinder@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#662371}
TBR=alph@chromium.org,einbinder@chromium.org,exterkamp@chromium.org,jeffish@microsoft.com,lomitch@microsoft.com
Change-Id: Ic4ca7d000d10b2bc61b585198469ca30f051dc62
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 941561
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627943
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#662798}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ceb872b6f6a02d707783563a225c77bb0c62170a
This change includes...
* A main GRD file for the DevTools (front_end/langpacks/devtools_ui_strings.grd)
* GRDP files organized by subfolder
* A node script that keeps the GRD/GRDP files in sync with the keys in the DevTools frontend
* A git cl presubmit --upload check that runs the node script
Note: Subsequent changes will add a build step to generate a .pak file that contains these DevTools strings. You can read about the overall approach here (https://bugs.chromium.org/p/chromium/issues/detail?id=941561).
Details of this design:
======================
We followed a similar pattern used by WebUI where strings are encoded in GRIT GRD/P files, which are used by a localization service to perform translations. They are also consumed in the build step to generate a .pak file, which is loaded by the browser's resource system.
Chromium documentation:
* https://www.chromium.org/developers/tools-we-use-in-chromium/grit/grit-users-guide
* https://www.chromium.org/developers/design-documents/ui-localization
Frontend strings:
-----------------
These are the localizable strings that are displayed to the user.
GRDP <message> strings:
-----------------------
Each frontend string has a corresponding <message> entry in a GRDP file. These entries are what the localization service will use to perform localizations. It's also the input to the GRIT compiler, which generates a .pak file, which is loaded by the browser's resource_bundle system.
GRDP <messsage> placeholders:
----------------------------
Frontend strings use placeholders, which are used to substitute in values at runtime.
For example,
'This string has %s two placeholder %.2f.'
Since the order of the string may change in a different language, we need to encode the order of the placeholders. As such, in the GRDP file you'll find %s replaced with $[1-9].
For example,
'This string has <ph name="ph1">$1s</ph> two placeholder <ph name="ph2">$2.2f</ph>.'
Also, note that the precision and type of the placeholder is maintained (i.e. .2f).
Detecting changes:
-----------------
The check_localizable_resources.js script performs the following check and generates an error if there are any changes that need to be made to a GRDP file.
1. Parses the frontend strings and hashes them.
2. Reads the messages from the GRDP files and hashes them.
3. Uses a difference between these two sets to report which strings need to be added and/or removed from GRDP files.
Optionally, the user can specify --autofix and it will automatically update the appropriate GRDP files.
Presubmit check:
---------------
Running git cl presubmit --upload will run the check_localizable_resources.js script with the --autofix argument.
If there are any changes, they reported to the user like this.
** Presubmit ERRORS **
Error: Found changes to localizable DevTools strings.
DevTools localizable resources checker has updated the appropriate grdp file(s).
Manually write a description for any new <message> entries.
Use git status to see what has changed.
BUG=941561
Change-Id: I5ac1656a037a6aaffeb4f64b103c4daec28be39a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613921
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#662371}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 2f05fdbf2ab3458b4b3a3f2a86f908a3a7d04f79
Mandy Chen