commit 49eda55ca4a14977e32197438cfa1fc0a00945cb
parent cb2357bac13971269b17b679f210fad9061d7629
Author: Alexandre Poirot <poirot.alex@gmail.com>
Date: Mon, 6 Oct 2025 12:40:33 +0000
Bug 1917012 - [devtools] Fix auto-pretty-printing preference implementation. r=devtools-reviewers,bomsy
Differential Revision: https://phabricator.services.mozilla.com/D221229
Diffstat:
10 files changed, 286 insertions(+), 53 deletions(-)
diff --git a/devtools/client/debugger/src/actions/sources/prettyPrint.js b/devtools/client/debugger/src/actions/sources/prettyPrint.js
@@ -11,7 +11,11 @@ import {
updateBreakpointsForNewPrettyPrintedSource,
} from "../breakpoints/index";
-import { getPrettySourceURL, isJavaScript } from "../../utils/source";
+import {
+ getPrettySourceURL,
+ isJavaScript,
+ isMinified,
+} from "../../utils/source";
import { isFulfilled, fulfilled } from "../../utils/async-value";
import {
getOriginalLocation,
@@ -31,6 +35,8 @@ import {
getFirstSourceActorForGeneratedSource,
getSource,
getSelectedLocation,
+ canPrettyPrintSource,
+ getSourceTextContentForSource,
} from "../../selectors/index";
import { selectSource } from "./select";
@@ -300,14 +306,20 @@ function selectPrettyLocation(prettySource) {
/**
* Toggle the pretty printing of a source's text.
- * Nothing will happen for non-javascript files.
+ * Nothing will happen for non-javascript, non-minified, or files that can't be pretty printed.
*
* @param Object source
* The source object for the minified/generated source.
+ * @param Boolean isAutoPrettyPrinting
+ * Are we pretty printing this source because of auto-pretty printing preference?
* @returns Promise
* A promise that resolves to the Pretty print/original source object.
*/
-export async function doPrettyPrintSource(source, thunkArgs) {
+export async function doPrettyPrintSource(
+ source,
+ isAutoPrettyPrinting,
+ thunkArgs
+) {
const { dispatch, getState } = thunkArgs;
recordEvent("pretty_print");
@@ -323,6 +335,25 @@ export async function doPrettyPrintSource(source, thunkArgs) {
await dispatch(loadGeneratedSourceText(sourceActor));
+ // Just after having retrieved the minimized text content,
+ // verify if the source can really be pretty printed.
+ // In case it can't, revert the pretty printed status on the minimized source.
+ // This is especially useful when automatic pretty printing is enabled.
+ if (
+ isAutoPrettyPrinting &&
+ (!canPrettyPrintSource(getState(), source, sourceActor) ||
+ !isMinified(
+ source,
+ getSourceTextContentForSource(getState(), source, sourceActor)
+ ))
+ ) {
+ dispatch({
+ type: "REMOVE_PRETTY_PRINTED_SOURCE",
+ source,
+ });
+ return null;
+ }
+
const newPrettySource = await dispatch(
createPrettySource(source, sourceActor)
);
@@ -351,13 +382,13 @@ export async function doPrettyPrintSource(source, thunkArgs) {
// Otherwise we may use generated frames there.
newPrettySource._loaded = true;
- return newPrettySource;
+ return fulfilled(newPrettySource);
}
// Use memoization in order to allow calling this actions many times
// while ensuring creating the pretty source only once.
export const prettyPrintSource = memoizeableAction("prettyPrintSource", {
- getValue: (source, { getState }) => {
+ getValue: ({ source }, { getState }) => {
// Lookup for an already existing pretty source
const url = getPrettyOriginalSourceURL(source);
const id = generatedToOriginalId(source.id, url);
@@ -368,13 +399,14 @@ export const prettyPrintSource = memoizeableAction("prettyPrintSource", {
}
return fulfilled(s);
},
- createKey: source => source.id,
- action: (source, thunkArgs) => doPrettyPrintSource(source, thunkArgs),
+ createKey: ({ source }) => source.id,
+ action: ({ source, isAutoPrettyPrinting = false }, thunkArgs) =>
+ doPrettyPrintSource(source, isAutoPrettyPrinting, thunkArgs),
});
export function prettyPrintAndSelectSource(source) {
return async ({ dispatch }) => {
- const prettySource = await dispatch(prettyPrintSource(source));
+ const prettySource = await dispatch(prettyPrintSource({ source }));
// Select the pretty/original source based on the location we may
// have had against the minified/generated source.
@@ -431,8 +463,8 @@ export function removePrettyPrintedSource(source) {
// Otherwise fallback to selectSource in order to select the first line instead of the current line within the pretty version.
if (location.source == generatedSource) {
await dispatch(selectSpecificLocation(location));
+ } else {
+ await dispatch(selectSource(generatedSource));
}
-
- await dispatch(selectSource(generatedSource));
};
}
diff --git a/devtools/client/debugger/src/actions/sources/select.js b/devtools/client/debugger/src/actions/sources/select.js
@@ -6,13 +6,12 @@
* Redux actions for the sources state
* @module actions/sources
*/
-import { prettyPrintSource, prettyPrintAndSelectSource } from "./prettyPrint";
-import { addTab, closeTabForSource } from "../tabs";
+import { prettyPrintSource } from "./prettyPrint";
+import { addTab } from "../tabs";
import { loadSourceText } from "./loadSourceText";
import { setBreakableLines } from "./breakableLines";
-
import { prefs } from "../../utils/prefs";
-import { isMinified } from "../../utils/source";
+
import { createLocation } from "../../utils/location";
import {
getRelatedMapLocation,
@@ -24,15 +23,13 @@ import {
getSource,
getFirstSourceActorForGeneratedSource,
getSourceByURL,
- getPrettySource,
getSelectedLocation,
getShouldSelectOriginalLocation,
- canPrettyPrintSource,
- getSourceTextContentForLocation,
tabExists,
hasSource,
hasSourceActor,
isPrettyPrinted,
+ isPrettyPrintedDisabled,
isSourceActorWithSourceMap,
getSelectedTraceIndex,
} from "../../selectors/index";
@@ -145,13 +142,27 @@ async function mayBeSelectMappedSource(location, keepContext, thunkArgs) {
// In this case we don't follow the "should select original location",
// we solely follow user decision to have pretty printed the source.
const sourceIsPrettyPrinted = isPrettyPrinted(getState(), location.source);
- if (!location.source.isOriginal && sourceIsPrettyPrinted) {
+ const shouldPrettyPrint =
+ !location.source.isOriginal &&
+ (sourceIsPrettyPrinted ||
+ (prefs.autoPrettyPrint &&
+ !isPrettyPrintedDisabled(getState(), location.source)));
+
+ if (shouldPrettyPrint) {
+ const isAutoPrettyPrinting =
+ !sourceIsPrettyPrinted && prefs.autoPrettyPrint;
// Note that prettyPrintSource has already been called a bit before when this generated source has been added
// but it is a slow operation and is most likely not resolved yet.
- // prettyPrintSource uses memoization to avoid doing the operation more than once, while waiting from both callsites.
+ // `prettyPrintSource` uses memoization to avoid doing the operation more than once, while waiting from both callsites.
const prettyPrintedSource = await dispatch(
- prettyPrintSource(location.source)
+ prettyPrintSource({ source: location.source, isAutoPrettyPrinting })
);
+
+ // Return to the current location if the source can't be pretty printed
+ if (!prettyPrintedSource) {
+ return { shouldSelectOriginalLocation, newLocation: location };
+ }
+
// If we aren't selecting a particular location line will be 0 and column be undefined,
// avoid calling getRelatedMapLocation which may not map to any original location.
if (location.line == 0 && !location.column) {
@@ -312,22 +323,6 @@ export function selectLocation(
return;
}
- const sourceTextContent = getSourceTextContentForLocation(
- getState(),
- location
- );
-
- if (
- keepContext &&
- prefs.autoPrettyPrint &&
- !getPrettySource(getState(), loadedSource.id) &&
- canPrettyPrintSource(getState(), location) &&
- isMinified(source, sourceTextContent)
- ) {
- await dispatch(prettyPrintAndSelectSource(loadedSource));
- dispatch(closeTabForSource(loadedSource));
- }
-
// When we select a generated source which has a sourcemap,
// asynchronously fetch the related original location in order to display
// the mapped location in the editor's footer.
diff --git a/devtools/client/debugger/src/components/Editor/Footer.js b/devtools/client/debugger/src/components/Editor/Footer.js
@@ -255,7 +255,10 @@ class SourceFooter extends PureComponent {
if (!this.props.isSourceActorWithSourceMap) {
return L10N.getStr("sourceFooter.sourceMapButton.sourceNotMapped");
}
- if (this.props.selectedLocation.source.isOriginal) {
+ if (
+ this.props.selectedLocation.source.isOriginal &&
+ !this.props.selectedLocation.source.isPrettyPrinted
+ ) {
return L10N.getStr("sourceFooter.sourceMapButton.isOriginalSource");
}
return L10N.getStr("sourceFooter.sourceMapButton.isBundleSource");
@@ -433,7 +436,8 @@ const mapStateToProps = state => {
// `mappedSource` will be null while loading, we need another way to know when it is done computing
!mappedSource &&
isSelectedMappedSourceLoading(state) &&
- !sourceMapError;
+ !sourceMapError &&
+ !selectedSource?.isPrettyPrinted;
return {
selectedSource,
@@ -453,7 +457,11 @@ const mapStateToProps = state => {
),
endPanelCollapsed: getPaneCollapse(state, "end"),
canPrettyPrint: selectedLocation
- ? canPrettyPrintSource(state, selectedLocation)
+ ? canPrettyPrintSource(
+ state,
+ selectedSource,
+ selectedLocation.sourceActor
+ )
: false,
prettyPrintMessage: selectedLocation
? getPrettyPrintMessage(state, selectedLocation)
diff --git a/devtools/client/debugger/src/reducers/tabs.js b/devtools/client/debugger/src/reducers/tabs.js
@@ -6,7 +6,12 @@
* Tabs reducer
*/
-export function initialTabState({ urls = [], prettyPrintedURLs = [] } = {}) {
+import { prefs } from "../utils/prefs";
+
+export function initialTabState({
+ urls = [],
+ prettyPrintedURLs = new Set(),
+} = {}) {
return {
// List of source URL's which should be automatically opened in a tab.
// This array will be stored as-is in the persisted async storage.
@@ -22,6 +27,12 @@ export function initialTabState({ urls = [], prettyPrintedURLs = [] } = {}) {
// (converted into Array in the asyncStorage)
prettyPrintedURLs,
+ // Similar but opposite of prettyPrintedURLs.
+ // List of sources which pretty printing has been manually disabled
+ // or are not considered minified when auto-pretty printing is enabled
+ // Set<String>
+ prettyPrintedDisabledURLs: new Set(),
+
// List of sources for which tabs should be currently displayed.
// This is transient data, specific to the current document and at this precise time.
//
@@ -76,9 +87,20 @@ function update(state = initialTabState(), action) {
* Allow unregistering pretty printed source earlier than source unregistering.
*/
function removePrettyPrintedSource(state, source) {
+ const generatedSourceURL = source.isPrettyPrinted
+ ? source.generatedSource.url
+ : source.url;
const prettyPrintedURLs = new Set(state.prettyPrintedURLs);
- prettyPrintedURLs.delete(source.generatedSource.url);
- return { ...state, prettyPrintedURLs };
+ prettyPrintedURLs.delete(generatedSourceURL);
+
+ let prettyPrintedDisabledURLs = state.prettyPrintedDisabledURLs;
+ // When auto-pretty printing is enabled, record sources which have been manually disabled
+ if (prefs.autoPrettyPrint) {
+ prettyPrintedDisabledURLs = new Set(prettyPrintedDisabledURLs);
+ prettyPrintedDisabledURLs.add(generatedSourceURL);
+ }
+
+ return { ...state, prettyPrintedURLs, prettyPrintedDisabledURLs };
}
/**
@@ -96,7 +118,8 @@ function removePrettyPrintedSource(state, source) {
* @return {Object} Modified state object
*/
function updateTabsWithNewActiveSource(state, sources, forceAdding = false) {
- let { urls, openedSources, prettyPrintedURLs } = state;
+ let { urls, openedSources, prettyPrintedURLs, prettyPrintedDisabledURLs } =
+ state;
for (let source of sources) {
// When we are adding a pretty printed source, we don't add a new tab.
// We only ensure the tab for the minimized/generated source is opened.
@@ -110,6 +133,7 @@ function updateTabsWithNewActiveSource(state, sources, forceAdding = false) {
prettyPrintedURLs = new Set(prettyPrintedURLs);
}
prettyPrintedURLs.add(source.url);
+ prettyPrintedDisabledURLs.delete(source.url);
}
const { url } = source;
@@ -167,9 +191,16 @@ function updateTabsWithNewActiveSource(state, sources, forceAdding = false) {
if (
openedSources != state.openedSources ||
urls != state.urls ||
- prettyPrintedURLs != state.prettyPrintedURLs
+ prettyPrintedURLs != state.prettyPrintedURLs ||
+ prettyPrintedDisabledURLs != state.prettyPrintedDisabledURLs
) {
- return { ...state, urls, openedSources, prettyPrintedURLs };
+ return {
+ ...state,
+ urls,
+ openedSources,
+ prettyPrintedURLs,
+ prettyPrintedDisabledURLs,
+ };
}
return state;
}
diff --git a/devtools/client/debugger/src/selectors/sources.js b/devtools/client/debugger/src/selectors/sources.js
@@ -18,7 +18,10 @@ import {
getBreakableLinesForSourceActors,
isSourceActorWithSourceMap,
} from "./source-actors";
-import { getSourceTextContentForLocation } from "./sources-content";
+import {
+ getSourceTextContentForLocation,
+ getSourceTextContentForSource,
+} from "./sources-content";
export function hasSource(state, id) {
return state.sources.mutableSources.has(id);
@@ -240,19 +243,17 @@ export function isSourceWithMap(state, id) {
return actors.some(actor => isSourceActorWithSourceMap(state, actor.id));
}
-export function canPrettyPrintSource(state, location) {
- const sourceId = location.source.id;
- const source = getSource(state, sourceId);
+export function canPrettyPrintSource(state, source, sourceActor) {
if (
!source ||
source.isPrettyPrinted ||
source.isOriginal ||
- (prefs.clientSourceMapsEnabled && isSourceWithMap(state, sourceId))
+ (prefs.clientSourceMapsEnabled && isSourceWithMap(state, source.id))
) {
return false;
}
- const content = getSourceTextContentForLocation(state, location);
+ const content = getSourceTextContentForSource(state, source, sourceActor);
const sourceContent = content && isFulfilled(content) ? content.value : null;
if (
diff --git a/devtools/client/debugger/src/selectors/tabs.js b/devtools/client/debugger/src/selectors/tabs.js
@@ -27,3 +27,11 @@ export function tabExists(state, source) {
export function isPrettyPrinted(state, source) {
return source.url && state.tabs.prettyPrintedURLs.has(source.url);
}
+
+/**
+ * Reports if a given source was ignored by auto-pretty printing,
+ * or if the user manually disabled pretty printing on it
+ */
+export function isPrettyPrintedDisabled(state, source) {
+ return source.url && state.tabs.prettyPrintedDisabledURLs.has(source.url);
+}
diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-auto.js b/devtools/client/debugger/test/mochitest/browser_dbg-pretty-print-auto.js
@@ -0,0 +1,153 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
+// Tests the auto pretty printing feature
+
+"use strict";
+
+add_task(async function testAutoPrettyPrintedOff() {
+ // Disable source map in order to allow pretty printing the bundle
+ await pushPref("devtools.source-map.client-service.enabled", false);
+
+ const dbg = await initDebugger("doc-sourcemaps3.html", "bundle.js");
+
+ // Expand the tree to make the source visible
+ await waitForSourcesInSourceTree(dbg, ["bundle.js"], {
+ noExpand: false,
+ });
+ // Do not use selectSource test helper as it ultimately pass keepContext=false to selectLocation action.
+ await selectSourceFromSourceTree(
+ dbg,
+ "bundle.js",
+ "Select the `bundle.js` script for the tree"
+ );
+
+ is(
+ findElement(dbg, "prettyPrintButton").disabled,
+ false,
+ "The pretty print button is enabled"
+ );
+});
+
+add_task(async function testAutoPrettyPrintedOn() {
+ // For now the preference is only read on debugger initialization, so close and re-open
+ // a debugger to see a change in behavior.
+ info("Enable auto pretty printing via the preference");
+ await pushPref("devtools.debugger.auto-pretty-print", true);
+
+ let dbg = await initDebugger("doc-sourcemaps3.html", "bundle.js");
+
+ // Expand the tree to make the source visible
+ await waitForSourcesInSourceTree(dbg, ["bundle.js"], {
+ noExpand: false,
+ });
+ // Do not use selectSource test helper as it ultimately pass keepContext=false to selectLocation action.
+ await selectSourceFromSourceTree(
+ dbg,
+ "bundle.js",
+ "Select the `bundle.js` script for the tree"
+ );
+
+ await waitForSelectedSource(dbg, "bundle.js");
+ ok(
+ findElement(dbg, "prettyPrintButton").classList.contains("pretty"),
+ "The pretty print button should report that the source has been pretty printed"
+ );
+
+ const source = findSource(dbg, "bundle.js:formatted");
+ await addBreakpoint(dbg, "bundle.js:formatted", 49);
+
+ invokeInTab("test");
+ await waitForPaused(dbg);
+
+ await assertPausedAtSourceAndLine(dbg, source.id, 49);
+
+ await stepOver(dbg);
+
+ await assertPausedAtSourceAndLine(dbg, source.id, 55);
+
+ await resume(dbg);
+
+ info("Reload and assert that the source keeps being pretty printed");
+ await reload(dbg, "bundle.js:formatted");
+ await waitForSelectedSource(dbg, "bundle.js");
+ ok(
+ findElement(dbg, "prettyPrintButton").classList.contains("pretty"),
+ "The source should still be pretty printed after reload"
+ );
+
+ // Close the tab and see if the source is also reopened directly to the pretty printed version
+ await closeTab(dbg, "bundle.js");
+
+ await closeTabAndToolbox();
+
+ // Do not use `initDebugger` as it resets all settings
+ const toolbox = await openNewTabAndToolbox(
+ EXAMPLE_URL + "doc-sourcemaps3.html",
+ "jsdebugger"
+ );
+ dbg = createDebuggerContext(toolbox);
+
+ await selectSourceFromSourceTree(
+ dbg,
+ "bundle.js",
+ "Select the `bundle.js` script for the tree"
+ );
+ ok(
+ findElement(dbg, "prettyPrintButton").classList.contains("pretty"),
+ "The source is still pretty printed after toolbox restart"
+ );
+
+ invokeInTab("test");
+ await waitForPaused(dbg);
+
+ await assertPausedAtSourceAndLine(dbg, source.id, 49);
+ is(dbg.selectors.getSelectedSource().isPrettyPrinted, true);
+
+ info("Manually disable pretty printing on the source");
+ await togglePrettyPrint(dbg);
+
+ info("Assert that we are showing the minimized source");
+ await assertPausedAtSourceAndLine(dbg, source.generatedSource.id, 1, 625);
+ is(dbg.selectors.getSelectedSource().isPrettyPrinted, false);
+
+ info("Assert that we keep showing the minimized source after steps");
+ await stepIn(dbg);
+ is(dbg.selectors.getSelectedSource().isPrettyPrinted, false);
+ await assertPausedAtSourceAndLine(dbg, source.generatedSource.id, 1, 655);
+
+ await resume(dbg);
+
+ info(
+ "Reload and assert that auto-pretty printing did *not* re-pretty printed the source"
+ );
+ await reload(dbg, "bundle.js");
+ ok(
+ !findElement(dbg, "prettyPrintButton").classList.contains("pretty"),
+ "The source should still *not* be pretty printed after reload"
+ );
+
+ info("Pause in an evaled source which shouldn't be pretty printed");
+ const onResumed = SpecialPowers.spawn(
+ gBrowser.selectedBrowser,
+ [],
+ async function () {
+ // Craft some source that makes `isMinified` function to consider the source as not minified
+ // thanks to the indentations
+ content.eval(`
+ function foo() {
+ debugger;
+ }
+ foo();`);
+ }
+ );
+ await waitForPaused(dbg);
+ ok(
+ !findElement(dbg, "prettyPrintButton").classList.contains("pretty"),
+ "Simple source should *not* be pretty printed"
+ );
+
+ await resume(dbg);
+ await onResumed;
+});
diff --git a/devtools/client/debugger/test/mochitest/browser_kz.toml b/devtools/client/debugger/test/mochitest/browser_kz.toml
@@ -86,6 +86,9 @@ fail-if = ["a11y_checks"] # Bug 1849028 clicked element may not be focusable and
["browser_dbg-paused-overlay.js"]
+["browser_dbg-pretty-print-auto.js"]
+fail-if = ["a11y_checks"] # Bug 1849028 clicked element may not be focusable and/or labeled
+
["browser_dbg-pretty-print-breakpoints-columns.js"]
fail-if = ["a11y_checks"] # Bug 1849028 clicked element may not be focusable and/or labeled
diff --git a/devtools/client/debugger/test/mochitest/shared-head.js b/devtools/client/debugger/test/mochitest/shared-head.js
@@ -1917,6 +1917,8 @@ const selectors = {
sourceTreeThreads: '.sources-list .tree-node[aria-level="1"]',
sourceTreeGroups: '.sources-list .tree-node[aria-level="2"]',
sourceTreeFiles: ".sources-list .tree-node[data-expandable=false]",
+ sourceTreeFilesElement: i =>
+ `.sources-list .tree-node[data-expandable=false]:nth-child(${i})`,
threadSourceTree: i => `.threads-list .sources-pane:nth-child(${i})`,
sourceDirectoryLabel: i => `.sources-list .tree-node:nth-child(${i}) .label`,
resultItems: ".result-list .result-item",
diff --git a/devtools/client/debugger/test/mochitest/sourcemaps/browser_dbg-sourcemaps-disabled.js b/devtools/client/debugger/test/mochitest/sourcemaps/browser_dbg-sourcemaps-disabled.js
@@ -39,8 +39,8 @@ add_task(async function () {
is(
footerButton.textContent,
- "original file",
- "The source map button now reports the pretty printed file as original file"
+ "bundle file",
+ "The source map button now reports the pretty printed file as bundle file (as we don't support toggling source map before reloading the page)"
);
ok(
!footerButton.classList.contains("disabled"),
