As part of the dark mode migration we are adding
`enableLegacyPatching:true` to all call sites of `appendStyle` (and
related methods). If a user passes `false` for that flag, no CSS colour
patching will occur.
This CL is therefore a no-op from a user's perspective as we pass `true`
on every call to maintain existing behaviour, but once we start
migrating we will turn the option to `false`. Long term, once all code
is migrated and the old patching is removed, we will remove this flag
entirely once again.
Bug: 1122511
Change-Id: I76b818e83b7d5ee0175e1548759373f53481e0ab
No-Presubmit: True
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2516345
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
As part of the initial seeding of the OWNERS, the automated scripts added
myself as OWNERS to pretty much every single folder in `front_end/`. This
is an artifact of the widespread infrastructure I have done, with in
particular the ES Modules migration that touched every single file in
DevTools.
In the past few weeks, several times I have received CLs were I was asked
to perform a domain review on the feature in question. Most of the time,
I have no knowledge of that particular feature.
To reduce confusion and roundtrips asking for different domain reviewers,
I removed myself as explicit OWNERS of these files. Note that I can still
use my INFRA_OWNERS to land changes, but as usual these are infra-only.
E.g. these will include the larger refactorings that span multiple features,
but not a single feature in particular.
I have kept myself OWNERS of folders that are part of the core
architecture of DevTools, as I am comfortable reviewing these as domain
reviewer.
R=yangguo@chromium.org,aerotwist@chromium.org
Change-Id: Ib1be70b97c5395498795b4b28ce7001731305d41
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2282516
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Previously, many CSS rules were written as:
-theme-preserve {
foo: bar;
}
Note that the selector is `-theme-preserve`, which in theory matches
any <-theme-preserve> elements — but we don’t use any such elements,
and it’s an invalid custom element name [1] anyway.
The only reason these selectors are used is to mark a CSS rule as
unaffected by DevTools theming; `_patchProperty` in `UIUtils.js` checks
if the selector contains `'-theme-'`.
stylelint-config-standard complains about this selector for good
reason, so we should use something else. A simple fix is to use a class
selector instead:
.-theme-preserve {
foo: bar;
}
There is one gotcha: `-theme-preserve` gets applied as an actual class
name to some <input> elements in the memory timeline under the
Performance panel. To prevent those elements from getting all the
`-theme-preserve` styles, this patch changes the class name for that
specific case to `-theme-preserve-input`.
This works great: stylelint is now happy, and `_patchProperty` in
`UIUtils.js` still works correctly, since it only checks if the
selector contains `-theme-` anywhere.
[1]: https://mothereff.in/custom-element-name#-theme-preserve
Bug: chromium:1083142
Change-Id: I923f12de6d581e9fef71a9a58bda184e40dee796
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2252017
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
This patch makes it so that all module.json files use the same
consistent indentation that matches .editorconfig and the rest
of the codebase. At the request of reviewers, one such file
(front_end/emulated_devices/module.json) has been excluded
from this patch. It will be reformatted in a separate CL.
Note that this patch does not “roll CodeMirror”, although a
pre-commit hook forces me to include that phrase in the commit
message because the module.json file in the relevant folder is
modified.
Bug: chromium:1070492
Change-Id: Ib31ed5232461e2f80bf05f31105fc919f6639b0a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2127007
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
We prefer single quotes by default, but still want to allow double
quotes in cases where that helps avoid quote-escaping, and also want
to allow template literals in cases where string interpolation is
used. For example:
const a = 'xxx'; // ok
const b = "xxx"; // not ok, should use single quotes
const c = "xxx'xxx"; // ok, double quotes avoid an escape sequence
const d = `xxx`; // not ok, frivolous use of template literal
const e = `xxx ${42}`; // ok
Per https://github.com/eslint/eslint/issues/12976, setting
the `allowTemplateLiterals` option for the `quotes` lint rule to
`false` gives us the desired behavior.
Note that the above lint rules are auto-fixed when running the linter;
there should be no need to manually make any changes to appease the
linter.
Per review feedback, this patch also removes the following escape
sequences for printable non-ASCII symbols:
- U+00D7 → ×
- U+2026 → …
- U+2019 → ’
Chromium CL temporarily updating test expectations:
https://chromium-review.googlesource.com/c/chromium/src/+/2083147
Cq-Depend: chromium:2083147
Bug: chromium:1057042
Change-Id: Id6bec3f96ca694d2fbc07dd8629fce305a58df8a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2082372
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Mathias Bynens <mathias@chromium.org>
This change makes the Changes drawer editor more accessible by adding a
gutter column with +/- symbols on added or deleted rows, respectively.
This information was previously conveyed only through color, a WCAG
violation.
In order to communicate whether a line is an addition or deletion to
screen reader users, I created a new code mirror input style that
prepends 'Addition:' or 'Deletion:' to the text that screen readers read
out for each diff line. This solution was settled on after discussing it
with other devs working on accessibility in order to avoid implementing
something more complex like VS Code's screen reader diff experience [1].
Screenshot: https://gyazo.com/8fe95b563a9e74d0d8b1a88900876c2a
[1] https://github.com/Microsoft/vscode/issues/17263#issuecomment-305701496
Bug: 963183
Change-Id: Ieeab30b8058fb36ef3a0d9ef4beac52419f6dc33
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1891836
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
This CL marks the list of files in the Changes drawer [1] as a tablist
rather than a tree. This provides clarity for screen reader users
because the current behavior better matches the Tab pattern as defined
in the WAI-ARIA Authoring Practices [2]:
> Tabs are a set of layered sections of content, known as tab panels,
> that display one panel of content at a time. Each tab panel has an
> associated tab element, that when activated, displays the panel.
> The list of tab elements is arranged along one edge of the
> currently displayed panel, most commonly the top edge.
[1] https://gyazo.com/12ee1829f0660b77523eb51bf59481e1
[2] https://www.w3.org/TR/wai-aria-practices-1.1/#tabpanel
Bug: 963183
Change-Id: Ic79c22ad55941e4b340c23c5979ec02adf60b080
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1891834
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
The Chromium/Google style guides does not enforce curly braces for
single-line if-statements, but does strongly recommend doing so. Adding
braces will improve code readability, by visually separating code
blocks. This will also prevent issues where accidental additions are
pushed to the "else"-clause instead of in the if-block.
This CL also updates the presubmit `eslint` to run the fix with the
correct configuration. It will now fix all issues it can fix.
Change-Id: I4b616f21a99393f168dec743c0bcbdc7f5db04a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821526
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jeff Fisher <jeffish@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#701070}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7e0bdbe2d7f9fc2386bfaefda3cc29c66ccc18f9
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
Accessibility testing revealed the following issues within the tool:
1. no focus indicator on javascript VM instances list
This change:
changes current selected/hover and adds selected but inactive color to match treeoutline in Element
Bug: 963183
Workflow gif: https://imgur.com/a/lHNTOP7
Change-Id: Iaed79d7166bf70011711be548ba66c65a57922da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1678946
Commit-Queue: Junyi Xiao <juxiao@microsoft.com>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#690483}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 522709bc5b3158fdaef516f391d382d7321e505f
This patch fills in the 'desc' tag for each <message> in the
changes tab with auto-generated descriptions.
Regarding the quality of these descriptions. Our plan is to fix
descriptions that result in poor translations as reported by our
translation team and end users.
Follow-up patches will be made to add descriptions to other grdp files.
Bug: 941561
Change-Id: I838baac8eb55f79a53e0f1e251713bc5a7237da1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1678842
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#672699}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 0a89851bb04e87065a9451bbe1acc5a964a8f1d0
Christy Chen