This was causing problems when trying to write a unit test that
included a file which transitively included formatter.js, which was
instantiating a SourceFormatter, leading to a chain of unfortunate
events which relied on some other module already being loaded, which
it wasn't.
To avoid the dependency chain between the imports, expose a function
getSourceFormatter which lazily instantiates a shared instance.
Bug: 1136848
Change-Id: I7c255186e53e131127ba00b7f719f2f1b34301c2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2456592
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Lexical declarations in `case` and `default` clauses are a footgun,
since they are visible in the entire switch block, but they only
get initialized upon assignment, which only happens if the relevant
`case` is actually reached.
To ensure that such lexical declarations only apply to the current
`case` (which is usually the intention), `case` clauses containing
them should be wrapped in curly braces to create an explicit block.
More information:
https://eslint.org/docs/rules/no-case-declarations
Change-Id: I63d9341fcd76d4b9ce8281bd0e6573b886577f08
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2119685
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
This CL fixes two similar issues where focus disappears or moves to
invisible objects. The first is that Performance panel timeline
currently keeps tabstops while blank, creating the appearance of focus
being lost after tabbing past the "Learn more" link. Hiding the
flamechart widget when it has nothing to show avoids focus moving to it.
In the coverage drawer, focus is lost completely when the user presses
the reload and record button. To avoid this, I've made the coverage
drawer check for focus when a recording begins and focus the datagrid if
the drawer has focus.
Video of performance working properly: https://i.imgur.com/OQKmeh9.mp4
Bug: 1055375
Change-Id: I553d48cad29611ae1828bc423f0d6ba7e7a6383e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2070680
Reviewed-by: Brian Cui <brcui@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
This CL updates the Datagrid so that it has the role of application.
This causes screen readers like NVDA to interact with it in an
application mode, similar to focus mode. It also adds the ability
for each node to specify how it wants its accessible text read, and
updates the accessible text for the datagrid on each selection or
on expanding/collapsing a node.
The goal of this change is that SR users wouldn't have to toggle modes
or get the massive content dump when hitting a datagrid.
Since grid is a div, a div receiving focus is expected by SR's to dump all
text unless role=applicaiton is set, which is not desirable for datagrids.
It is the recommended approach
for custom div sections that require specific user interaction
to mark role="application" and specify through custom labels how SR
users should interact. https://github.com/nvaccess/nvda/issues/7360
This is the same approach used by google sheets and excel online.
Additionally, SRs that support changing modes can still navigate
the table in browse mode by changing modes after the
datagrid receives focus.
More references:
http://blog.jantrid.net/2015/12/woe-aria-surprisingly-but-ridiculously.htmlhttps://github.com/nvaccess/nvda/issues/7807https://www.davidmacd.com/blog/does-aria-label-override-static-text.html
Change-Id: I7c563fbdb358e3f68e62bac1c3fefcddb8f65307
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1967346
Reviewed-by: Vidal Diazleal <vidorteg@microsoft.com>
Reviewed-by: Amanda Baker <ambake@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
Commit-Queue: Brandon Goddard <brgoddar@microsoft.com>
Issue:
- Data grids in DevTools do not have an aria-label
- There are a ton of DataGrid parameters and adding one is messy
Changes:
- Naming all data grids
- Created DataGrid.Datagrid.Parameters type to hold data grid parameters (including a required gridName field)
Bug: 963183
Change-Id: I83b130d468fb80034be264b45b86b799f650e978
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1891652
Commit-Queue: Michael Liao <michael.liao@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
- The value of an <option> is a string, not a number, hence block
coverage could not be selected.
- JavaScriptPerBlock is misleading; the value is a flag that indicates
that JavaScript coverage is present (and presence of
JavaScriptPerFunction indicates that only per function coverage is
present).
Bug: chromium:1004203
Change-Id: Ib59fd94c8cace48cafcf62b179fa492531379a7e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1899510
Reviewed-by: Jan Scheffler <janscheffler@chromium.org>
Commit-Queue: Jan Scheffler <janscheffler@chromium.org>
Per block coverage is far more expensive to collect, so we
opt for per function coverage for now. We plan on bringing
back block coverage at some point as an additional choice.
The main draw-back of the more coarse-grained per function
coverage is that inline scripts are now reported as covered
as soon as their first line was executed (because inline
scripts are treated like a function on the V8 side).
Here is a comparison. Note how the inline scripts in the
index file now show higher coverage.
Before/After: https://imgur.com/a/WnEXCWs
Bug: chromium:1009396
Change-Id: I1c0da7b45c099206028f08300d7c3a92b10a4433
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1880038
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
This change adds accessible text to columns in the coverage tool
that were not previously screen reader accessible. It adds accessible text
for the Total bytes column to prevent the numbers from being read separately
due to the spaces. It adds a label to the Unused Bytes column to prevent
screen readers from interpreting the column as 1 large percentage
instead of separate bytes value and percentage values.
And it adds accessible text to describe the bars
in the last column. Text reads
"X% of file unused. Y% of file used."
Before: https://imgur.com/qpAwxVF
After: https://imgur.com/GTqrstQ
Bug: 963183
Change-Id: I2b69cc32e935f94d2138f7964f449a4f0472182a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710753
Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Brandon Goddard <brgoddar@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#707530}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c7b23afaf3b42c471cd6a30b6f8d40759e617591
The source editor, when it cannot load a particular file, will simply
display a blank file instead. At the bottom, this is because the APIs
used to load the files do not have a way to propagate an error up to
the call site. Rather, the callback either is never called, or is
called with just an empty string.
This refactor changes the way the project system loads file contents
by replacing the callback model with a Promise-based model. However,
in this version, rather than propagating an error (handled via
catch), the error is exposed as a property on the object passed via
the the load functions. Although it might be preferable to use
async throw/catch, because there are ~4-5 layers of redirection
through the project system, the added complexity seems to not really
justify that work. I'm open to reconsidering this design, though.
Attempting to load a file via file:// which does not exist previously
produced no error because the DevToolsUIBindings handler would just
always resolve with no content and HTTP status 200. I had previously
addressed that bug in this changeset, but I've split it out to
https://chromium-review.googlesource.com/c/chromium/src/+/1847833 .
Sample "after" screenshot: https://imgur.com/a/tlm90sg
Bug: 961940
Bug: 941035
Change-Id: If121611090e9c35eeb1de162b59f8a9f72f696d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1817677
Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: Jeff Fisher <jeffish@microsoft.com>
Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#705438}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 61ec2a233d9a7bf4bb4b3e610de6fc233ae0d46e
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
Peter Marshall