We are fixing mochitests that fail when network.cookie.cookieBehavior = 5, i.e. when we enable Total Cookie Protection.
This is most often due to the test assuming that an origin will always have access to its storage state when embedded as
a third party.
My approach: Add third-party storage permission for the net domain when included on the com domain.
This allows the cache for the net domain to be shared in the way that this test is expecting.
Differential Revision: https://phabricator.services.mozilla.com/D136601
We don't need to have specific check for dom0 event listeners as the debugger
button should be show as long as we managed to create a source actor.
We can remove usage of `script.source.element` alltogether as we don't need
it for not showing the line number in the location (the line is 0 in such case,
and we already discriminate for such value).
A test case is added to ensure that the debugger can be opened from the
event tooltip on a dom0 event.
Differential Revision: https://phabricator.services.mozilla.com/D136461
This is only an issue when devtools.popups.debug is false,
when it is true, the toolbox is move and a warning message is printed
if we request to open a new toolbox.
Differential Revision: https://phabricator.services.mozilla.com/D136498
We were missing cached image request and probably CSP blocked request
from the parent process because of this.
We are spawning a NetworkEventContentWatcher for the parent process target.
That's semi-intentional. This might better be done via a parent process resource type,
like NETWORK_EVENT. But the current Resource framework doesn't allow to do that easily.
So that we currently spawn the NetworkEventContentWatcher (and NetworkEventStackTraceWatcher)
against the parent process target actor as a "FRAME" resource type.
This is handy as we need at least one of these two watchers for the parent process.
Differential Revision: https://phabricator.services.mozilla.com/D136325
It was only used in storeExpressions, where we can replace it by storing only the input property
instead of removing the value property from the object we store.
Differential Revision: https://phabricator.services.mozilla.com/D136164
test_stepping-12.js wasn't correctly waiting for thread to be resumed between each of its sub test.
Similarly, other tests weren't waiting for thread front's resume request to be resume.
I'm also spreading the usage of resumeAndWaitForPause to better ensure we resume and wait for next paused event
without any race condition. Listening to "paused" after calling "resume" is subject to race conditions.
Differential Revision: https://phabricator.services.mozilla.com/D136125
If a test involving the Browser Toolbox times out, the Browser Toolbox
would not close and could have impact on next tests.
In this patch we register a cleanup function to make sure we always
close the Browser Toolbox.
Differential Revision: https://phabricator.services.mozilla.com/D136096
We take this opportunity to rename `removeInnerLocations` to `getInnerLocations`
and make it return an array of inner locations rather than returning the list
of original locations with inner locations stripped.
This makes the function easier to understand (not having to deal with indexes
+ splicing), and doesn't harm the only callsite where we can handle those inner
locations into the filter call that was done on the array returned from `removeInnerLocations`
The parser-worker was updated.
Differential Revision: https://phabricator.services.mozilla.com/D136095
If the browser toolbox is started from a Marionette enabled process,
Marionette should never be started for the browser toolbox process.
If we do as right now the browser toolbox process will immediately
shutdown because Marionette cannot listen on the already in-use
socket port and forces a process shutdown.
Differential Revision: https://phabricator.services.mozilla.com/D136220
This helps:
* registering blackboxing before the page starts loading.
So that blackboxing better work if we break early during the page load.
* transfert the blackboxing information only once per URL instead of once per source actor.
We now communicate the ranges once to the BlackboxingActor.
Also, we can pass a list of ranges instead of range per range.
Differential Revision: https://phabricator.services.mozilla.com/D135614
The icon is now a crossed-out file and is displayed in red.
We set the background for ignored line to a red-ish grey that will hopefully
better convey the fact that those lines are ignored.
Differential Revision: https://phabricator.services.mozilla.com/D135974
With this patch on its own we get some a11y tests failures, but those
are fixed on a later patch.
Combobox select no longer creates frames for its <options>, nor an
nsListControlFrame. Instead, it computes its right intrinsic size using
the largest size of the options. This is better, because we render the
option text using the select style so if the select and option styles
are mismatched it'd cause changes in the size of the select when text
changes. See the following in a build without the patch, for example:
<select>
<option>ABC</option>
<option style="font-size: 1px">Something long</option>
</select>
This seems like a rather obscure case, but it's important to get it
right, see bug 1741888.
With this patch we use the same setup in content and parent processes
(this needs bug 1596852 and bug 1744152). This means we can remove a
bunch of the native view and popup code in nsListControlFrame. A couple
browser_* tests are affected by this change and have been tweaked
appropriately (the changes there are trivial).
Not creating an nsListControlFrame for dropdown select means that we
need to move a bunch of the event handling code from nsListControlFrame
to a common place that nsComboboxControlFrame can also use. That place
is HTMLSelectEventListener, and I think the setup is much nicer than
having the code intertwined with nsListControlFrame. It should be
relatively straight-forward to review, mostly moving code from one part
to another.
Another thing that we need to do in HTMLSelectEventListener that we
didn't use to do is listening for DOM mutations on the dropdown. Before,
we were relying on changes like text mutations triggering a reflow of
the listcontrolframe, which also triggered a reflow of the
comboboxcontrolframe, which in turn updated the text of the anonymous
content. Now we need to trigger that reflow manually.
There are some further simplifications that can be done after this
lands (cleanup naming of openInParentProcess and so on, among others),
but I'd rather land this first (after the merge of course) and work on
them separately.
Differential Revision: https://phabricator.services.mozilla.com/D132719
This will cause a new highlighter to be created when onToggleFontHighlight
is called next.
Ideally, we wouldn't have a reference to the highlighter as this is probably
not working for Fission, and we would use the HighlightersOverlay like the
other inspector panel, but that probably requires its own bug.
A test is added to ensure this works as expected.
Differential Revision: https://phabricator.services.mozilla.com/D135998
With this patch on its own we get some a11y tests failures, but those
are fixed on a later patch.
Combobox select no longer creates frames for its <options>, nor an
nsListControlFrame. Instead, it computes its right intrinsic size using
the largest size of the options. This is better, because we render the
option text using the select style so if the select and option styles
are mismatched it'd cause changes in the size of the select when text
changes. See the following in a build without the patch, for example:
<select>
<option>ABC</option>
<option style="font-size: 1px">Something long</option>
</select>
This seems like a rather obscure case, but it's important to get it
right, see bug 1741888.
With this patch we use the same setup in content and parent processes
(this needs bug 1596852 and bug 1744152). This means we can remove a
bunch of the native view and popup code in nsListControlFrame. A couple
browser_* tests are affected by this change and have been tweaked
appropriately (the changes there are trivial).
Not creating an nsListControlFrame for dropdown select means that we
need to move a bunch of the event handling code from nsListControlFrame
to a common place that nsComboboxControlFrame can also use. That place
is HTMLSelectEventListener, and I think the setup is much nicer than
having the code intertwined with nsListControlFrame. It should be
relatively straight-forward to review, mostly moving code from one part
to another.
Another thing that we need to do in HTMLSelectEventListener that we
didn't use to do is listening for DOM mutations on the dropdown. Before,
we were relying on changes like text mutations triggering a reflow of
the listcontrolframe, which also triggered a reflow of the
comboboxcontrolframe, which in turn updated the text of the anonymous
content. Now we need to trigger that reflow manually.
There are some further simplifications that can be done after this
lands (cleanup naming of openInParentProcess and so on, among others),
but I'd rather land this first (after the merge of course) and work on
them separately.
Differential Revision: https://phabricator.services.mozilla.com/D132719
Doing this helps load resource-command module from jest tests
which were throwing when loading the transformers.
And it probably is a small performance improvement.
Differential Revision: https://phabricator.services.mozilla.com/D135719
This is also behind "devtools.popups.debug", to be set to true manually.
This special mode break the old fundamental principal where a given toolbox
is bound to one unique given tab. Now one toolbox starts being shared between
many tabs.
When we select the original tab we debug, or any of its popups opened in distinct tabs,
we will move the toolbox between each of these tabs.
We will have one toolbox instance, one toolbox iframe, which will be moved
around each tab's host. This is somewhat similar to host switching within the same tab.
This is all based on the same trick where we swap the toolbox iframe to another location.
Differential Revision: https://phabricator.services.mozilla.com/D131802
With popup debugging (next patches), we trigger a race condition in this code
where `SourcesManager.urlContents` is called *after* `devtools-html-content` is fired.
i.e. after the HTML document is parsed.
This lead to return an async promise instead of an immediate value.
This confuses `SourceActor._getStartLineColumnDisplacement` which no longer apply breakpoints right away.
We miss early breakpoint support for popups.
This isn't easy to reproduce beyond popup debugging,
in next changeset, browser_dbg-breakpoints-popup.js's testPausedByBreakpoint covers this.
Differential Revision: https://phabricator.services.mozilla.com/D135144
For now, we only do that when "devtools.popups.debug" is manually set to true.
This is introducing some complexity in the way we filter out the WindowGlobal
we should consider or not. Before this patch it was quite straightforward.
We accepted all WindowGlobal's matching the tab's `browserId`.
Now we also accept the WindowGlobal whose `opener`'s `browserId` matches.
With this patch only, popups start appearing in the iframe dropdown.
You still have to manually switch to the popup via the dropdown to debug it in the inspector or console.
In the debugger, you will already start seeing the popup source and break on it.
Differential Revision: https://phabricator.services.mozilla.com/D133350
This patch adds a checkbox at the end of each event listeners in the EventTooltip,
which allow the user to disable/re-enable a given event listener.
This is done by managing a Map of nsIEventListenerInfo object in the NodeActor,
which we populate from `getEventListenerInfo`. Each `nsIEventListenerInfo` is
assigned a generated id, which can then be used to call the new NodeActor
methods, `(enable|disable)EventListener`.
We don't support disabling jquery/React event listeners at the moment, so we
display the checkbox for them as well, but disabled.
Differential Revision: https://phabricator.services.mozilla.com/D135133
This patch adds a checkbox at the end of each event listeners in the EventTooltip,
which allow the user to disable/re-enable a given event listener.
This is done by managing a Map of nsIEventListenerInfo object in the NodeActor,
which we populate from `getEventListenerInfo`. Each `nsIEventListenerInfo` is
assigned a generated id, which can then be used to call the new NodeActor
methods, `(enable|disable)EventListener`.
We don't support disabling jquery/React event listeners at the moment, so we
display the checkbox for them as well, but disabled.
Differential Revision: https://phabricator.services.mozilla.com/D135133
Unfortunately, I haven't find any useful attribute on JSWindowActorChild/WindowGlobalChild/BrowsingContext
to detect that things are destroyed, or to be destroyed and avoid calling sendAsyncMessage,
or detect in the exception handler that we got destroyed.
So I'm falling back to ignore the exception based on its message...
Differential Revision: https://phabricator.services.mozilla.com/D135608
In the Browser Console, document-event resources are retrieved from a
legacy listener built on top of the documentEvent event, sent by the
webconsole actor.
documentEvent wasn't including hasNativeConsoleAPI, and we were displaying
a misleading overriden console API message there.
This patch fixes this and adds a test case to make sure we don't regress.
Funnily, the mochitest test harness _does_ override the global's console
property, and we have to reset it in the test to make sure the warning message
is not displayed.
Differential Revision: https://phabricator.services.mozilla.com/D135585
Track all code which may filter BrowsingContext or WindowGlobal in the server codebase
in order to use a unique filtering method.
Differential Revision: https://phabricator.services.mozilla.com/D134423
Popup debugging (bug 1569859) will force to revisit how we filter out the BrowsingContext
that are meant to be debugged. We won't only accept BrowsingContext based on their browserId.
This would force us to carefuly review all the codes where we filter BrowsingContexts.
And if we later have to tweak this, do this again.
It would be nice to have a unique method to filter things out.
It will also be beneficial once we add new debuggable contexts like workers
as we would only have to tweak this method.
For now, this patch focuses only on Target helpers and JSWindowActor's,
but I'll followup to other server modules.
Note that I'm changing the behavior of getAllRemoteBrowsingContexts
in order to also return the top browsing context by default.
We were having a few places where we were re-adding it after,
but that's not trivial. It is easier to remove it in the rare function that need that.
Differential Revision: https://phabricator.services.mozilla.com/D134422
This boolean helps know for which BrowsingContext we should create a target or track resources.
So that it is part of what defines the context we should debug and will be handy to have
in all filtering functions we use to filter browsing context or platform objects.
Differential Revision: https://phabricator.services.mozilla.com/D134421
This commit adds support for setting the pseudo-locale for the browser
UI directly from the Browser Toolbox. This places the icons in the same
place as the "Disable Popup Autohide" command. This will make it easier
for Firefox developers to test that their UI is properly localized.
The SVGs were optimized for size using an optimizer that dropped the
path precision and any extra tags. I tested that they work correctly in
both light and dark modes.
Pseudo-localization is documented here:
https://firefox-source-docs.mozilla.org/l10n/fluent/tutorial.html#pseudolocalization
After this patch lands I'll follow-up with updating that documentation.
Differential Revision: https://phabricator.services.mozilla.com/D134420
In target-actor-mixin, we were calling `setActiveEventBreakpoints` only with the
new events we were receiving, which mean if the user was clicking a first event in
the UI, and then a second one, only the second one would have a functioning event breakpoint.
Also, we were not handling removing event breakpoints at all.
We're adding `(add|remove)EventBreakpoints` to the thread actor so it's easier
to update the list of event breakpoints.
The existing event breakpoints test is modified to ensure we don't regress such behaviour.
The call to `setEventListenerBreakpoints` is moved before dispatch the `UPDATE_EVENT_LISTENERS`
action so we can properly wait for the breakpoint to be set in tests.
Differential Revision: https://phabricator.services.mozilla.com/D135216
"original" here refers to not being generated.
It overlaps with pretty printed sources, which are considered as original sources.
Differential Revision: https://phabricator.services.mozilla.com/D134751
And better document all types of source objects:
* generated
* source map original sources
* pretty printed sources
As well as source object attributes!
Differential Revision: https://phabricator.services.mozilla.com/D134786
Avoids using context menu, affected by bug 1478596, so that the test can be
re-enabled in Linux WebRender.
Replaces deprecated OS.File with IOUtils.
Adds try..catch as an attempt to investigate timeouts like bug 1650268.
Adds some extra checks, and some refactorings.
Differential Revision: https://phabricator.services.mozilla.com/D135146
parentMap appears to only be used when pressing the left arrow key to navigate
to the parent folder in the source tree. If this is too slow it could be
replaced with a traverseTree search.
Differential Revision: https://phabricator.services.mozilla.com/D115318
The information it provides is unclear and I don't see how this could be valuable.
We take this opportunity to move the tags before the "capturing" label to get
better alignment between mixed events (e.g. regular and React ones).
Differential Revision: https://phabricator.services.mozilla.com/D135130
- Remove `NodeActor#getEventListeners` which was simply a proxy to `EventCollector#getEventListeners`
- Change signature of `EventCollector#processHandlerForEvent` to make code easier to follow
Differential Revision: https://phabricator.services.mozilla.com/D135128
The function was only creating an EventTooltip instance, so we can directly modify
the only call site to do the same thing.
Since the EventTooltip isn't responsible for showing the tooltip itself, and given
that the consumer code already does some work when the tooltip gets hidden, we
let the consumer call EventTooltip#destroy instead of having the EventTooltip
register the event listener on the tooltip.
Differential Revision: https://phabricator.services.mozilla.com/D135127
HAR export was still producing empty har files if any of the request
currently displayed in the netmonitor can't be completely retrieved.
This includes all lazy data. But if any request has been destroyed in the server
fetching any lazy data will throw.
Differential Revision: https://phabricator.services.mozilla.com/D134789
This is more reliable check and it feels more common type of check
we already do elsewhere.
It also helps expose less convoluted objects to NetworkEventActor and so simplify it.
Differential Revision: https://phabricator.services.mozilla.com/D133474
This will be useful in next patch to know what is the overall debugged context.
We can finally know if that's the browser context/toolbox from almost any part of the server codebase!
Differential Revision: https://phabricator.services.mozilla.com/D133472
In prevision of a broader use of this existing "context" object,
I'm renaming it in order to make this important object clearer and more easily identifiable.
SessionContext object defines what we are debugging (everything, only one tab, an addon,...)
and so controls which platform data we should expose back to the client.
Differential Revision: https://phabricator.services.mozilla.com/D134411
We were notifying about same-process storage values twice.
Once with the top document target and another time via the iframe target.
This was probably invisible from the UI. We were probably only updating things twice without any visible breakage.
But this also highlighted that the test was quite wrong.
It was only asserting about the last resource for each storage type.
Luckily, the last resource to be notified was the one from the top document.
So that it contained all the storage values about all the iframes.
When I fix EFT implementation, now, iframe data is only visible in the iframe's related storage resource.
I tweaked the test to correctly assert data against the many resources for a single storage type.
Differential Revision: https://phabricator.services.mozilla.com/D134378
This test was brittle as it was assuming that only one resource was notified per storage type.
But this wasn't the case since fission or EFT!
We were lucky that the assertion were correct only against the latest resource
and not against the aggregate of the many resource (one per target).
Differential Revision: https://phabricator.services.mozilla.com/D134376
Actually I was wrong in bug 1745795.
This wasn't SourceFront but SOURCE resources, which are SourceActor's form and not a Front.
Let's make it clear and simplify the code some more.
Also stop calling makeSourceId from create.js
as we already have the reducer's source id via the reducer's source-actor's source attribute.
Differential Revision: https://phabricator.services.mozilla.com/D134006
SourceQueue used to throttle both generated and original sources.
But since we migrated to ResourceCommand, and it comes with a builtin throttling,
we only use SourceQueue for original sources.
So we can acknowledge that and simplify this a bit.
Differential Revision: https://phabricator.services.mozilla.com/D134004
* SourceActor's introductionUrl has been removed in Fx77 in bug 1607639.
* getBreakableLines has been added in Fx68 in bug 1537779.
* getAvailableEventBreakpoints changed to return an array in Fx69 in bug 1549999.
Differential Revision: https://phabricator.services.mozilla.com/D134003
Color incrementation now also works for color functions following the CSS Color 4 specification, i.e. with spaces as separators and the alpha value separated by a slash.
The test case related to incrementation of color values also got extended to cover this change plus the color functions hsl(), hsla() and hwb().
Differential Revision: https://phabricator.services.mozilla.com/D132993
This change doesn't cover auto-completion for "hwb" yet because it isn't added automatically when generating the properties-db.js.
Differential Revision: https://phabricator.services.mozilla.com/D132992
The keyboard shortcut was supposed to move to the next search
result, but was only working on OSX. As you can already cycle
through search results with Enter, and given that Cmd+G only (barely)
work on OSX, it's safe to remove it.
Differential Revision: https://phabricator.services.mozilla.com/D134128
This updates profiler-get-symbols to the latest version.
The new /source/v1 entry point was implemented in
edb78205d1
Furthermore, this update also picks up a fix for macOS 12 system library symbolication.
Differential Revision: https://phabricator.services.mozilla.com/D133757
Basic UI functionality for blackboxing
Enable this feature pref `devtools.debugger.features.blackbox-lines`
This is mainly to gather feeback from the team and webcompat, things
like the selection colors will likely change, maybe we can add a context menu
to the editor gutter as well etc.
Differential Revision: https://phabricator.services.mozilla.com/D132368
This patch includes the following:-
- The reducer logic to store the blackbox url and range info matches that on the
server here https://searchfox.org/mozilla-central/rev/dfc0dea63a16b73078a46b6ae49b2a626b8c11b5/devtools/server/actors/utils/sources-manager.js#289-335
Note: The client logic supports multiple ranges whhile the server supports one range (maybe we can add support for multiple to the server).
- Added jest tests for some edge cases
- Changed the asyncStore pref from `tabsBlackBoxed` to `blackboxedTabs` for readability ease
and to avoid and errors as the format of the data stored changes.
Before: `asyncStore.tabsBlackBoxed = ["foo.com", "bar.com"...]`
After: `asyncStore.blackboxedTabs = { "foo.com": [], "bar.com": [range, range, ...] }`
The range sturcture
```
const range = {
start: { line: 1, column: 5 },
end: { line: 3, column: 4 },
}
```
Differential Revision: https://phabricator.services.mozilla.com/D132774
These exception may happen when some pending request from the previous page
are received lated by the netmonitor, which tries to fetch extra data about these
old requests.
On the server side, these requests are destroyed when devtools.netmonitor.persistlog=false.
(which is the default behavior)
Differential Revision: https://phabricator.services.mozilla.com/D132495
In this patch we need to ensure that all NETWORK resources work correctly for the browser toolbox.
The main fix was to ensure that all Network resources were emitted by browsingContextID set to -1,
otherwise the ResourceCommand may try to lookup for the related target and won't find it.
That's because the browser toolbox uses a unique target for all parent process requests,
and still doesn't use EFT. So same-process iframe should still be debugged by their parent document target.
Also ResourceCommand used isServerTargetSwitchingEnabled on descriptor front,
but for now, only TabDescriptorFront exposes this method.
Differential Revision: https://phabricator.services.mozilla.com/D103770
This watcher wasn't filtering request from http-on-image-cache-response
And request froms http-on-failed-opening-request were using a different filtering code
compared to network-events-stacktraces.js
So now, all network event related resources should be using the exact same filtering.
Then may be filtering by window isn't the best. It might actually be better to filter
by innerWindowId or browsingContextID. But this is another story and thanks
to this match, it will be easier to migrate all these codes to a new filtering.
Also, I had to tweak getWindowForRequest in order to correctly return the window
for the CSS request done in test_network_get.html.
This will probably fix some missing stack trace for some requests like this one!
Differential Revision: https://phabricator.services.mozilla.com/D132989
In this test, there is two that are actually dedicated to cover ResourceCommand internal cache
and the precise behavior of ignoreExistingResources: true/false.
I pulled them off in a dedicated test file and tried to simplified them a bit.
Because the server doesn't really support fetch request done before calling watchResources,
this test is a bit hard to understand. So I tried to make it clear via extensive comments.
Then, my goal is to reuse the third and simplier test to extend the coverage of NETWORK_EVENT
with many other assertions.
Differential Revision: https://phabricator.services.mozilla.com/D133768
I'm also removing the `isBlackBoxed` boolean from source actor in the reducer
in order to ensure we always only use the source reducer one.
And I dropped some comment here and there to help understand where all these sources objects are coming from.
Differential Revision: https://phabricator.services.mozilla.com/D133658
This patch creates an HTTP Custom request component without functionality, only shows the new panel on the left. Also, it adds a simple test, that will be uptaded as new features are added.
Differential Revision: https://phabricator.services.mozilla.com/D133390
This patch looks bigger than it is, but it's mostly because
of plumbing.
To implement revert-layer we need not only the cascade origin of the
declaration, but the whole cascade level, plus also the layer order.
In order to do this, encapsulate these two things inside a 32-bit
`CascadePriority` struct and plumb it through the rule tree and so on.
This allows us to remove the packing and unpacking of CascadeLevel,
though I kept the ShadowCascadeOrder limit for now in case we need to
reintroduce it.
Fix `!important` behavior of layers while at it (implementing it in
`CascadeLevel::cmp`, spec quote included since it was tricky to find)
since some revert-layer tests were depending on it.
The style attribute test is failing now, but follow-up commit fixes
it, see spec issue.
In terms of the actual keyword implementation, it's sort of
straight-forward: We implement revert and revert-layer in a shared
way, by storing the cascade priority that reverted it.
Differential Revision: https://phabricator.services.mozilla.com/D133372
I've elected to rename the function from `Should...` to
`ShouldNot...`, but not to rename the various test files. The
functionality under test is both "should" and "should not", so I think
the churn of renaming is not justified.
This rearranges the deck chairs to accommodate testing the new
functionality in the next commit.
Differential Revision: https://phabricator.services.mozilla.com/D133556
I've elected to rename the function from `Should...` to
`ShouldNot...`, but not to rename the various test files. The
functionality under test is both "should" and "should not", so I think
the churn of renaming is not justified.
This rearranges the deck chairs to accommodate testing the new
functionality in the next commit.
Differential Revision: https://phabricator.services.mozilla.com/D133556
This patch does a couple things to tell users they can use the node picker when
debugging a remote page:
- changes the node picker button icon (arrow -> hand) when connected to an android phone,
as well as its tooltip
- adds a notice in the content page (i.e. on the phone) when the picker starts
Differential Revision: https://phabricator.services.mozilla.com/D132705
This code could be useful in other places, especially in tests. (See next changeset)
I'm even wondering if `forMainProcess` should be using such type of client by default?
Concretely, in production, when we debug the main process, I think we always use such special client/server.
Differential Revision: https://phabricator.services.mozilla.com/D133188