This reverts commit 25a9b3bd7404d49e55772ffba046ed8df65ad14e.
Reason for revert: Memory snapshots work again
Original change's description:
> Revert "[DevTools]: Moves common to ES modules"
>
> This reverts commit e064553334d4b0c925f0b1f0e2e0234be66b71be.
>
> Reason for revert: Memory tool fails to take heap snapshots with this
> error: https://imgur.com/hzR9Eqw
>
> Original change's description:
> > [DevTools]: Moves common to ES modules
> >
> > This CL converts the contents of the common/ folder over to ES modules
> > from the current custom module system. It includes statements in each
> > file that will pollute the global namespace, allowing the non-ESM code
> > to continue working as before.
> >
> > Bug: 1006759
> > Change-Id: Ib591e49155000381f85999a5da6c467ad0160996
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1816563
> > Reviewed-by: Yang Guo <yangguo@chromium.org>
> > Commit-Queue: Paul Lewis <aerotwist@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#699728}
>
>
> Change-Id: Ic0d526780451e8bb4de538f11c85859a3badfb0f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1006759
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825463
> Commit-Queue: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#699835}
Bug: 1006759
Change-Id: I48c7e84262bb295e5830cac8a79a274226df046c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825420
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#699981}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 75a39ca7ac8dcfc6a830db07a5a389ca9ab27b1d
This CL converts the contents of the common/ folder over to ES modules
from the current custom module system. It includes statements in each
file that will pollute the global namespace, allowing the non-ESM code
to continue working as before.
Bug: 1006759
Change-Id: Ib591e49155000381f85999a5da6c467ad0160996
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1816563
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#699728}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: e064553334d4b0c925f0b1f0e2e0234be66b71be
The backend changes[1] for localizing devtools leverage the existing
webui functionalities, which consume a list of id mappings from the id
of a string used in the frontend (i.e. the English string) to the IDS_
key of the string in the corresponding grdp file. Example:
const LocalizedString kLocalizedStrings[] = {
...
{"Editable", IDS_DEVTOOLS_2c09e798bf8c5a82d211331c82893a0f},
...
}
This patch adds scripts that generate the structure. The output will be
two files, devtools_ui_strings_map.h and .cc, under
<generated files root directory>/chrome/browser/ui/webui
(e.g. D:/chromium/src/out/Release/gen/chrome/browser/ui/webui).
The design is to create a build step that calls generate_devtools_ui_strings.py,
which then calls generate_devtools_ui_strings.js. Integration into the build
system will come as a another patch, because it depends on the first set of
changes[2].
Example arguments to generate_devtools_ui_strings.js to generate output
files (invoke under '--root_gen_dir', e.g. D:/chromium/src/out/release/gen
in the example below):
['D:/chromium/src/third_party/node/win/node.exe',
'D:/chromium/src/third_party/blink/renderer/devtools/scripts/build/generate_devtools_ui_strings.js',
'--root_gen_dir', 'D:/chromium/src/out/release/gen',
'--output_header', 'chrome/browser/ui/webui/devtools_ui_strings_map.h',
'--output_cc', 'chrome/browser/ui/webui/devtools_ui_strings_map.cc']
[1]: https://crrev.com/c/1672628
[2]: https://crrev.com/c/1628927
Bug: 941561
Change-Id: Ic03dae24cd074219790c6eec5df3183c24bf15a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1671848
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#699370}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 3927e5494ba1866261dc577d8f93660fd9f04b5d
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
Closure was unhappy with the `Object.assign` solution. Instead, use
individual assignments and add the relevant comments, so that Closure
can understand what is going on.
Sample output:
```
/*Legacy exported object*/
self.UI = self.UI || {};
/*Legacy exported object*/
UI = UI || {};
//TODO(http://crbug.com/1006759): Add type information if necessary
UI.TextEditor = TextEditor;
//TODO(http://crbug.com/1006759): Add type information if necessary
UI.TextEditor.prototype = prototype;
/** @enum {symbol} */
UI.TextEditor.Events = Events;
```
Bug: 1006759
Change-Id: Ib0e571cc027f544b316a3e3ccf8c3d606273d887
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819248
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Tim Van der Lippe <tvanderlippe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#698878}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c1c9f965bfcf2dbcd8f868414874c085f4ff9515
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
This reverts commit 901bcc219d9204748f9c256ceca0f2cd68061006.
Reason for revert: ESLint errors fixed
Original change's description:
> Revert "Reland "Convert ui/ARIAUtils.js to an ES module""
>
> This reverts commit ec810baced8e07ca3e9f2b801703be433807d2a2.
>
> Reason for revert: causing eslint failures on Linux Tests:
> /b/s/w/ir/third_party/blink/renderer/devtools/front_end/dom_extension/DOMExtension.js
> 817:10 error 'onInvokeElement' is defined but never used no-unused-vars
> https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests/81950
>
> Original change's description:
> > Reland "Convert ui/ARIAUtils.js to an ES module"
> >
> > This is a reland of b3373919f86df10c190a8672ca828c7ea4b920e5
> >
> > Original change's description:
> > > Convert ui/ARIAUtils.js to an ES module
> > >
> > > ES modules allow files to explicitly import and export symbols [1].
> > > This patch migrates ARIAUtils.js to an ES module, while maintaining
> > > backwards compatibility with existing usages of `window.UI.ARIAUtils`.
> > >
> > > The build system is updated to copy these modules verbatim, as it no
> > > longer require pre-processing to be loaded. The `module.json` no longer
> > > includes this file as a script, to make sure it is not double-bundled
> > > into `shell.js`. Instead, it is added to the new field `modules`, which
> > > contains all modules. By doing so, it becomes clear which files are
> > > converted to modules already and which require the legacy references
> > > to the global scope.
> > >
> > > This module and future modules will be imported from `root.js`, which is
> > > added to all existing HTML application files.
> > >
> > > Design Doc: https://docs.google.com/document/d/1h9dOy3nNPNfZ2AtZXzB-DwJqG4Oo37WWvKLCuzCcPzo/edit?usp=sharing
> > >
> > > [1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
> > >
> > > TBR=jochen@chromium.org
> > >
> > > Change-Id: Ie79c8f2fce3aff96fa28af0b575eae39bfe8e1a5
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1748949
> > > Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> > > Reviewed-by: Yang Guo <yangguo@chromium.org>
> > > Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
> > > Cr-Commit-Position: refs/heads/master@{#697238}
> >
> > Change-Id: I7544cbf4364d0b1a3c2f137a4d17a1afdb1d4aed
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810534
> > Reviewed-by: Yang Guo <yangguo@chromium.org>
> > Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#697626}
>
> TBR=yangguo@chromium.org,aerotwist@chromium.org,tvanderlippe@chromium.org
>
> Change-Id: I445fd35f135450cd9e8ed4c69bf913662f461ce6
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1811989
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#697824}
Bug: 1004860
Change-Id: Idac8108b1b47488faa08473d2450a7ee0bdde96e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1813503
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#698029}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 4adc7b63ea9ea093206c62363c8fdf50fc64aed8
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
Example invocation of the script:
./refactor-folder-to-es-modules.sh ui
It will consequently refactor all files in the ui folder to JavaScript
modules. It will also update the BUILD.gn variables to remove the
references from the old all_devtools_files into the new modules
variables.
Change-Id: I35ad6360b15d1aff250afffc8a28774c4e8cc1ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808867
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#697602}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 6d48f36bc75c558ac1ee899ccb20877b800e9410
The new compiler caught a lot of pre-existing issues in the codebase.
Sadly, the old compiler version was not smart enough to understand the
new changes. Therefore, the changes have be included in the same CL as
the compiler update.
Most of the changes are related to better handling of prototype and
class inheritance, as well as handling of null/undefined tracking.
Change-Id: I3941a3a240a4d09c4945e1e20d2521090ef837c9
Bug: 991710
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762081
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
Auto-Submit: Tim van der Lippe <tvanderlippe@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#696761}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ca93474213278e32e36d6ace1474c56884030757
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
This reverts commit 9a2b5dd02a4ef572145214f4808df3dc2e93b1e6.
The assignment on line 45 was overwriting the parameter `src`, which
then would result in the for-loop on line 44 to traverse into the wrong
directory. Renaming the assignment to variable `new_src` resolves that
issue.
Confirmed working locally by commenting out the `if symlink exists`
branch and ran it on my Linux machine. (I could reproduce the error by
simply commenting and can confirm that it is now working with the fix)
TBR=luoe@chromium.org
Change-Id: Idc3dbddd9db620169f7e9fbd60e306755eb1b4c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715303
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Erik Luo <luoe@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#681390}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 01ff6dc78f8b331b7e76d026b553c0eb4b9a7322
This reverts commit a66c49ab42280aaaa4cf2b293b8e0c42c545b154.
Reason for revert: This breaks building chromium on Windows:
Traceback (most recent call last):
File "../../third_party/blink/renderer/devtools/scripts/build/build_debug_applications.py", line 72, in <module>
sys.exit(main(sys.argv))
File "../../third_party/blink/renderer/devtools/scripts/build/build_debug_applications.py", line 32, in main
symlink_dir_or_copy(input_path, output_path)
File "../../third_party/blink/renderer/devtools/scripts/build/build_debug_applications.py", line 49, in symlink_dir_or_copy
copy_file(src, join(dest, filename), safe=True)
File "../../third_party/blink/renderer/devtools/scripts/build/build_debug_applications.py", line 54, in copy_file
os.remove(dest)
WindowsError: [Error 5] Access is denied: 'resources/inspector/debug\\accessibility'
Original change's description:
> Symlink debug folder instead of symlinking individual files
>
> The original behavior of build_debug_applications.py was to traverse all
> files recursively in the front_end folder and symlink each individual
> file to the version in third_party/blink/renderer/devtools/front_end.
>
> However, this requires rebuilding of Chrome when new files are added to
> the application. After this CL, the developer experience is more
> What-You-See-Is-What-You-Get.
>
> This CL is also in preparation to remove the logic in C++-land to remove
> the need for a separate debug folder. A follow-up CL will change the
> behavior of `debug_devtools` to symlink directly into
> out/[NAME]/resources/inspector, rather than an explicit sub-directory.
>
> As a nice side-effect of this change, we are no longer symlinking the
> debug folder for every version of the application. Previously, it would
> iterate through all `application_names` and symlink the full debug
> folder. This is time-consuming and did not provide any actual benefit.
> Since we now symlink directly to the local version, we no longer need to
> build the HTML files directly.
>
> Bug: 986365
> Change-Id: I7e54ac3af57e0a3941b88a46b71b3ddacd5664f2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713344
> Reviewed-by: Erik Luo <luoe@chromium.org>
> Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
> Cr-Commit-Position: refs/heads/master@{#680518}
Change-Id: I8511095bb6f3dc8e4e7871afd3ef8c91fa63a069
Bug: 986365
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717594
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Jeff Fisher <jeffish@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#680679}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 9a2b5dd02a4ef572145214f4808df3dc2e93b1e6
The original behavior of build_debug_applications.py was to traverse all
files recursively in the front_end folder and symlink each individual
file to the version in third_party/blink/renderer/devtools/front_end.
However, this requires rebuilding of Chrome when new files are added to
the application. After this CL, the developer experience is more
What-You-See-Is-What-You-Get.
This CL is also in preparation to remove the logic in C++-land to remove
the need for a separate debug folder. A follow-up CL will change the
behavior of `debug_devtools` to symlink directly into
out/[NAME]/resources/inspector, rather than an explicit sub-directory.
As a nice side-effect of this change, we are no longer symlinking the
debug folder for every version of the application. Previously, it would
iterate through all `application_names` and symlink the full debug
folder. This is time-consuming and did not provide any actual benefit.
Since we now symlink directly to the local version, we no longer need to
build the HTML files directly.
Bug: 986365
Change-Id: I7e54ac3af57e0a3941b88a46b71b3ddacd5664f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713344
Reviewed-by: Erik Luo <luoe@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#680518}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: a66c49ab42280aaaa4cf2b293b8e0c42c545b154
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
Add a localization concatenation check to the presubmit script that issues
an error when a disallowed concatenation is found. Concatenation to a
localizable string is only allowed when it's a string that doesn't
contain letters.
* Example (allowed): ls`Status code` + ': '
* Example (disallowed): ls`Status code: ` + statusCode
* Example (disallowed): ls`Status ` + 'code'
I also fixed the current violations.
Bug: 941561
Change-Id: I428a174f00ad4567a280756b13cfb44bc317d75a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637146
Reviewed-by: Erik Luo <luoe@chromium.org>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#666084}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 5ee0394c3f084d01f0c4a4de2f9b5b455c0db40a
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
Manually wrap localizable strings in localization calls and remove the
presubmit check that issues an error when the 3rd argument to
createInput() is not localized.
Presubmit output before:
DevTools localization checker detected errors!
third_party\blink\renderer\devtools\front_end\devices\DevicesView.js Line 379: third argument to createInput() should be localized: editor.createInput('port', 'text', 'Device port (3333)', portValidator.bind(this))
third_party\blink\renderer\devtools\front_end\devices\DevicesView.js Line 382: third argument to createInput() should be localized: editor.createInput('address', 'text', 'Local address (dev.example.corp:3333)', addressValidator)
third_party\blink\renderer\devtools\front_end\network\NetworkManageCustomHeadersView.js Line 136: third argument to createInput() should be localized: editor.createInput('header', 'text', 'x-custom-header', validateHeader.bind(this))
third_party\blink\renderer\devtools\front_end\node_main\NodeConnectionsPanel.js Line 166: third argument to createInput() should be localized: editor.createInput('address', 'text', 'Network address (e.g. localhost:9229)', addressValidator)
third_party\blink\renderer\devtools\front_end\persistence\EditFileSystemView.js Line 159: third argument to createInput() should be localized: editor.createInput('pathPrefix', 'text', '/path/to/folder/', pathPrefixValidator.bind(this))
third_party\blink\renderer\devtools\front_end\settings\FrameworkBlackboxSettingsTab.js Line 137: third argument to createInput() should be localized: editor.createInput('pattern', 'text', '/framework\\.js$', patternValidator.bind(this))
After:
DevTools localization checker passed
Bug: 965674
Change-Id: Ie3d8f0b20283c0997d00e42bd8341af22c10b32f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623461
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#662299}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: b855f4a2b3f306bb3b9e5e2797ac49adfe5438aa
These css rules used classes that don't exist anywhere in the
javascript. A script was written to find them, but manually checking
is still needed because some scripts generate class names by
string concatenation like 'icon-' + iconType
Change-Id: I15c4f26451b9fa371b366a8977021e2af7eca449
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618043
Commit-Queue: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Erik Luo <luoe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#661630}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: e6f259d6e2834c9b0e2cbb0fdd7c9703e1fc6968
devtools:// scheme
This change replaces the chrome-devtools:// URL scheme, with a more
browser-agnostic equivalent.
Naturally, many files had to be touched, but it's mostly a
straightforward find/replace.
A related change is out against the catapult repo here:
https://chromium-review.googlesource.com/c/catapult/+/1606268,
which removes a test file containing the old scheme. Once it goes in,
the root DEPS file in the chromium repo needs to be updated as well.
Note:
This is a continuation of patch 1552143, but since Fabio is out on
leave and I don't have permissions to upload to his patch, I needed to
start a new one.
Bug: 960409
Change-Id: I3ede7f8d1dcbca457167eb3641ae60e9685d8d78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1582881
Commit-Queue: James Lissiak <jalissia@microsoft.com>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#659944}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 28b21a67f09a13c465b73776aecd9995d8a07737
Discovered a bug in check_gn.js, which is part of the presubmit check that
verifies all the devtools frontend files are included in devtools/BUILD.gn.
The bug prevents missing module.json files from being reported.
Output of running npm run check-gn
Before the fix:
DevTools BUILD.gn checker passed
After:
DevTools BUILD.gn checker detected errors!
There's an issue with: D:\chromium\src\third_party\blink\renderer\devtools\BUILD.gn
Missing file in BUILD.gn for all_devtools_files: "front_end\elements_test_runner\module.json",
Missing file in BUILD.gn for all_devtools_files: "front_end\persistence\module.json",
Missing file in BUILD.gn for all_devtools_files: "front_end\search\module.json",
Missing file in BUILD.gn for all_devtools_files: "front_end\services\module.json",
Missing file in BUILD.gn for all_devtools_files: "front_end\terminal\module.json",
Missing file in BUILD.gn for all_devtools_files: "front_end\worker_service\module.json",
I added these missing module.json files to BUILD.gn.
Change-Id: I05a72e353475af142c13f0644fd430361edfff72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1576409
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#653018}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 66c0e97960fed83511d320eded9f028e1cb678ad
* Enabled the check_localization presubmit script.
* Fixed localization issues caught by the check_localization presubmit script.
* Updated the check_localization script to allow contatenation of non-alphabetic strings with localized strings.
* For example, ls`Status Code` + ": " is a valid concatenation. This allows for decorations to be concatenated with localized strings.
Change-Id: I741940c9ebdac363ac0ccad3f7de20d508204e2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544878
Auto-Submit: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#647134}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7a329cb73b0b5d5cddb89a34fd7da6cb1a3c87dd
After submiting accessbility fixes for the Sensors tool and Performance
Monitor, we should aim to not regress these improvements.
This change adds the AxeCoreTestRunner module which uses aXe to run
basic accessbility checks on the UI.
It also adds a test that runs aXe against the Performance Monitor and
Sensors tool since they have already received an initial accessibility
pass and are not reporting any violations in aXe.
Change-Id: I6443d5014d9200a9295746aaec7a71b4659a6417
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1534551
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Amanda Baker <ambake@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#645823}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: f854b6f5a7a1e16fe5880c40e8055e92eca37943
This PR adds devtools frontend localizability check to the presubmit
script as reqeusted in
https://chromium-review.googlesource.com/c/chromium/src/+/1504254.
The new script, check_localizability.js, which run on frontend files
changed as part of the devtools presubmit check. In addition, you can
run $ npm run check-loc inside devtools/ to run the check on all
frontend js files.
**Note: Errors identified will be fixed and submitted as a separate
PR. **
check_localizability.js usage:
$ node check_localizability.js [-a | <.js file path>*]
-a: scan all devtools frontend js files
<.js file path>*: List of .js files with space-separated absolute paths
Localizability violations that it scans for:
- First argument to Common.UIString / UI.formatLocalized is not a string
- Conditional argument (e.g. cond ? trueCase : falseCase) found in
Common.UIString / UI.formatLocalized / ls tagged template literal
- String concatenation is used with
Common.UIString / UI.formatLocalized / ls``
- A specific argument to function is not localized (i.e. not wrapped in
Common.UIString / UI.formatLocalized / ls``)
- Currently, the script checks the third argument to createInput()
Here are a few examples of errors the script can output:
third_party\blink\renderer\devtools\front_end\resources\IndexedDBViews.js
Line 1456: string concatenation should be changed to variable
substitution with ls: ' (' + Common.UIString('Key path: ')
third_party\blink\renderer\devtools\front_end\timeline\TimelineUIUtils.js
Line 559-560: conditional(s) found in Common.UIString(
'%s [%s\u2026%s]', url, event.args['beginData']['startLine'] + 1,
endLine >= 0 ? endLine + 1 : ''). Please extract conditional(s) out
of the localization call.
Change-Id: I3243787ad5920b9b873f2feac7ed50a9c69bbe7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524993
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#643161}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: f1c8d4f5752ed8ab43babb602fa86121f8b14dbb
Paul Lewis