From e5cfa82bcdf962df23b2f89e901990f6bb6cd66e Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 13 Aug 2026 11:12:13 -0400 Subject: [PATCH 1/9] Publish panel: key the sanitized profile encoding state by sharing mode. The Download and Share panels have independent sharing options, so they sanitize to different profiles. sanitizePII always allocates a new Profile (even its no-op early return does {...profile, shared}), so the two modes never produce an identical object. The encoding state was a single slot keyed on that identity, which meant: - Each panel open missed both reuse checks and clobbered the other mode's entry, so an untouched profile was re-serialized and re-gzipped on every single open, forever. Alternating opens never settled. - The panel rendered whatever blob was in the shared slot, with no check that it had been encoded for its own mode's options. Opting into URLs in Share and then opening Download left a DONE blob containing those URLs sitting in the slot. What kept that from being offered was only that componentDidMount dispatches ENCODING synchronously before paint -- an accidental invariant, not a designed one. Give each mode its own slot and thread the mode through the three encoding actions. mapStateToProps now selects by ownProps.mode, so a panel is structurally incapable of seeing the other mode's blob, and each mode's encoding survives reopening. --- src/actions/publish.ts | 16 ++++-- src/components/app/MenuButtons/Publish.tsx | 5 +- src/reducers/publish.ts | 55 +++++++++++++------ src/selectors/publish.ts | 11 +++- src/test/store/publish.test.ts | 64 +++++++++++++++++++++- src/types/actions.ts | 3 + src/types/state.ts | 9 ++- 7 files changed, 137 insertions(+), 26 deletions(-) diff --git a/src/actions/publish.ts b/src/actions/publish.ts index f4b3869bd2..87eb45d66c 100644 --- a/src/actions/publish.ts +++ b/src/actions/publish.ts @@ -78,33 +78,39 @@ export function updateSharingOption( } export function sanitizedProfileEncodingStarted( + mode: SharingMode, sanitizedProfile: Profile, encodingPromise: Promise ): Action { return { type: 'SANITIZED_PROFILE_ENCODING_STARTED', + mode, sanitizedProfile, encodingPromise, }; } export function sanitizedProfileEncodingCompleted( + mode: SharingMode, sanitizedProfile: Profile, profileData: Blob ): Action { return { type: 'SANITIZED_PROFILE_ENCODING_COMPLETED', + mode, sanitizedProfile, profileData, }; } export function sanitizedProfileEncodingFailed( + mode: SharingMode, sanitizedProfile: Profile, error: Error ): Action { return { type: 'SANITIZED_PROFILE_ENCODING_FAILED', + mode, sanitizedProfile, error, }; @@ -295,7 +301,7 @@ export function encodeSanitizedProfile( const state = getState(); const sanitizedProfile = getSanitizedProfile(state, mode).profile; - const encodingState = getSanitizedProfileEncodingState(state); + const encodingState = getSanitizedProfileEncodingState(state, mode); if ( encodingState.phase === 'ENCODING' && encodingState.sanitizedProfile === sanitizedProfile @@ -331,17 +337,19 @@ export function encodeSanitizedProfile( serializeProfileToJsonString(sanitizedProfile) ); const blob = new Blob([gzipData], { type: 'application/octet-binary' }); - dispatch(sanitizedProfileEncodingCompleted(sanitizedProfile, blob)); + dispatch( + sanitizedProfileEncodingCompleted(mode, sanitizedProfile, blob) + ); return { type: 'SUCCESS', profileData: blob }; } catch (error) { - dispatch(sanitizedProfileEncodingFailed(sanitizedProfile, error)); + dispatch(sanitizedProfileEncodingFailed(mode, sanitizedProfile, error)); console.error('Error while compressing the profile data', error); return { type: 'ERROR', error }; } })(); dispatch( - sanitizedProfileEncodingStarted(sanitizedProfile, encodingPromise) + sanitizedProfileEncodingStarted(mode, sanitizedProfile, encodingPromise) ); return { sanitizedProfile, encodingPromise }; }; diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 413b8c49f6..3cce582090 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -425,7 +425,10 @@ export const PublishPanel = explicitConnect< profileHasJSTracingArgumentValues: getHasJSTracingArgumentValues(state), checkedSharingOptions: getCheckedSharingOptions(state, ownProps.mode), downloadFileName: getFilenameString(state), - sanitizedProfileEncodingState: getSanitizedProfileEncodingState(state), + sanitizedProfileEncodingState: getSanitizedProfileEncodingState( + state, + ownProps.mode + ), uploadPhase: getUploadPhase(state), uploadProgress: getUploadProgress(state), uploadProgressString: getUploadProgressString(state), diff --git a/src/reducers/publish.ts b/src/reducers/publish.ts index 896d1e4cc8..5a6259dbfb 100644 --- a/src/reducers/publish.ts +++ b/src/reducers/publish.ts @@ -46,8 +46,10 @@ function _getMostlyNonSanitizingSharingOptions(): CheckedSharingOptions { }; } -// Both modes reference the same object until one is edited, so an untouched -// profile is only encoded once. +// Both modes start out referencing the same options object. Note that this +// does not make them share an encoding: the sanitized profile is memoized per +// mode, and sanitizePII always returns a fresh object, so each mode encodes +// its own copy. See sanitizedProfileEncodingStates below. function _sharingOptionsForBothModes( options: CheckedSharingOptions ): Record { @@ -230,32 +232,53 @@ const hasSanitizedProfile: Reducer = (state = false, action) => { } }; -const sanitizedProfileEncodingState: Reducer = ( - state = { phase: 'INITIAL' }, +type EncodingStates = Record; + +const INITIAL_ENCODING_STATES: EncodingStates = { + download: { phase: 'INITIAL' }, + upload: { phase: 'INITIAL' }, +}; + +// One slot per sharing mode. Keying by mode keeps each panel's encoding +// stable across reopens, and guarantees a panel can never be handed a blob +// that was encoded for the other mode's sharing options. +const sanitizedProfileEncodingStates: Reducer = ( + state = INITIAL_ENCODING_STATES, action -): SanitizedProfileEncodingState => { +): EncodingStates => { switch (action.type) { case 'SANITIZED_PROFILE_ENCODING_STARTED': { - const { sanitizedProfile, encodingPromise } = action; - return { phase: 'ENCODING', sanitizedProfile, encodingPromise }; + const { mode, sanitizedProfile, encodingPromise } = action; + return { + ...state, + [mode]: { phase: 'ENCODING', sanitizedProfile, encodingPromise }, + }; } case 'SANITIZED_PROFILE_ENCODING_COMPLETED': { - const { sanitizedProfile, profileData } = action; + const { mode, sanitizedProfile, profileData } = action; + const current = state[mode]; if ( - state.phase === 'ENCODING' && - state.sanitizedProfile === sanitizedProfile + current.phase === 'ENCODING' && + current.sanitizedProfile === sanitizedProfile ) { - return { phase: 'DONE', sanitizedProfile, profileData }; + return { + ...state, + [mode]: { phase: 'DONE', sanitizedProfile, profileData }, + }; } return state; // Ignore updates from earlier encodings. } case 'SANITIZED_PROFILE_ENCODING_FAILED': { - const { sanitizedProfile, error } = action; + const { mode, sanitizedProfile, error } = action; + const current = state[mode]; if ( - state.phase === 'ENCODING' && - state.sanitizedProfile === sanitizedProfile + current.phase === 'ENCODING' && + current.sanitizedProfile === sanitizedProfile ) { - return { phase: 'ERROR', sanitizedProfile, error }; + return { + ...state, + [mode]: { phase: 'ERROR', sanitizedProfile, error }, + }; } return state; // Ignore updates from earlier encodings. } @@ -266,7 +289,7 @@ const sanitizedProfileEncodingState: Reducer = ( const publishReducer: Reducer = combineReducers({ checkedSharingOptions, - sanitizedProfileEncodingState, + sanitizedProfileEncodingStates, upload, isHidingStaleProfile, hasSanitizedProfile, diff --git a/src/selectors/publish.ts b/src/selectors/publish.ts index fe3d1aa103..cd6d92380c 100644 --- a/src/selectors/publish.ts +++ b/src/selectors/publish.ts @@ -275,9 +275,14 @@ export const getSanitizedProfile: DangerousSelectorWithArguments< SharingMode > = (state, mode) => _sanitizedProfileSelectors[mode](state); -export const getSanitizedProfileEncodingState: Selector< - SanitizedProfileEncodingState -> = (state) => getPublishState(state).sanitizedProfileEncodingState; +export const getSanitizedProfileEncodingStates: Selector< + Record +> = (state) => getPublishState(state).sanitizedProfileEncodingStates; + +export const getSanitizedProfileEncodingState: DangerousSelectorWithArguments< + SanitizedProfileEncodingState, + SharingMode +> = (state, mode) => getSanitizedProfileEncodingStates(state)[mode]; export const getUploadState: Selector = (state) => getPublishState(state).upload; diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index a38d10b558..e8a235518c 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -23,6 +23,7 @@ import { getUploadProgress, getUploadProgressString, getUploadGeneration, + getSanitizedProfileEncodingState, } from '../../selectors/publish'; import { getUrlState, @@ -68,7 +69,7 @@ import { listAllUploadedProfileInformationFromDb, } from 'firefox-profiler/app-logic/uploaded-profiles-db'; -import type { Store, UploadPhase } from 'firefox-profiler/types'; +import type { Store, UploadPhase, SharingMode } from 'firefox-profiler/types'; import { autoMockIndexedDB } from 'firefox-profiler/test/fixtures/mocks/indexeddb'; autoMockIndexedDB(); @@ -192,6 +193,67 @@ describe('encodeSanitizedProfile', function () { expect(second.encodingPromise).toBe(first.encodingPromise); await first.encodingPromise; }); + + it('keeps a separate encoding per mode, so alternating panel opens each reuse their own', async function () { + const { profile } = getProfileFromTextSamples('A'); + const { dispatch } = storeWithProfile(profile); + + // The two modes sanitize to different profiles, so they each need their own + // encoding... + const download = dispatch(encodeSanitizedProfile('download')); + const upload = dispatch(encodeSanitizedProfile('upload')); + expect(upload.encodingPromise).not.toBe(download.encodingPromise); + + // ...but reopening a panel must not start a third and fourth encoding. + expect(dispatch(encodeSanitizedProfile('download')).encodingPromise).toBe( + download.encodingPromise + ); + expect(dispatch(encodeSanitizedProfile('upload')).encodingPromise).toBe( + upload.encodingPromise + ); + + await Promise.all([download.encodingPromise, upload.encodingPromise]); + }); + + it('never exposes a mode the blob that was encoded for the other mode', async function () { + const { profile } = getProfileFromTextSamples('A'); + profile.pages = [ + { + tabID: 1, + innerWindowID: 1, + url: 'https://example.com/private-page', + embedderInnerWindowID: 0, + }, + ]; + const { dispatch, getState } = storeWithProfile(profile); + + // Opt into URLs for uploading only, and let that encoding finish. + dispatch(updateSharingOption('upload', 'includeUrls', true)); + await dispatch(encodeSanitizedProfile('upload')).encodingPromise; + expect(getSanitizedProfileEncodingState(getState(), 'upload').phase).toBe( + 'DONE' + ); + + // The download panel, whose options still strip URLs, must not see it. + const forDownload = getSanitizedProfileEncodingState( + getState(), + 'download' + ); + expect(forDownload.phase).toBe('INITIAL'); + + // And once download has encoded too, each mode keeps its own blob. + await dispatch(encodeSanitizedProfile('download')).encodingPromise; + const encodedBlob = (mode: SharingMode) => { + const encodingState = getSanitizedProfileEncodingState(getState(), mode); + if (encodingState.phase !== 'DONE') { + throw new Error( + `Expected the ${mode} encoding to be DONE, but it is ${encodingState.phase}.` + ); + } + return encodingState.profileData; + }; + expect(encodedBlob('download')).not.toBe(encodedBlob('upload')); + }); }); describe('getRemoveProfileInformation', function () { diff --git a/src/types/actions.ts b/src/types/actions.ts index 888a3b05a9..1a6ec0a82c 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -656,16 +656,19 @@ type PublishAction = } | { readonly type: 'SANITIZED_PROFILE_ENCODING_STARTED'; + readonly mode: SharingMode; readonly sanitizedProfile: Profile; readonly encodingPromise: Promise; } | { readonly type: 'SANITIZED_PROFILE_ENCODING_COMPLETED'; + readonly mode: SharingMode; readonly sanitizedProfile: Profile; readonly profileData: Blob; } | { readonly type: 'SANITIZED_PROFILE_ENCODING_FAILED'; + readonly mode: SharingMode; readonly sanitizedProfile: Profile; readonly error: Error; } diff --git a/src/types/state.ts b/src/types/state.ts index 5583e72181..e01956a84c 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -239,7 +239,14 @@ export type SanitizedProfileEncodingState = export type PublishState = { readonly checkedSharingOptions: Record; - readonly sanitizedProfileEncodingState: SanitizedProfileEncodingState; + // One encoding slot per sharing mode. The two modes have independent + // sharing options, so they sanitize to different profiles and must not + // share a slot: otherwise each mode's encoding clobbers the other's, and a + // panel could offer a blob that was encoded for the *other* mode's options. + readonly sanitizedProfileEncodingStates: Record< + SharingMode, + SanitizedProfileEncodingState + >; readonly upload: UploadState; readonly isHidingStaleProfile: boolean; readonly hasSanitizedProfile: boolean; From b29cfe2abd9d7da70e6e25a81a49dccd63f10af9 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 13 Aug 2026 11:17:15 -0400 Subject: [PATCH 2/9] Publish panel: return just the encoding promise from encodeSanitizedProfile. --- src/actions/publish.ts | 34 +++++++++++----------------------- src/test/store/publish.test.ts | 20 ++++++++------------ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/actions/publish.ts b/src/actions/publish.ts index 87eb45d66c..f787762b23 100644 --- a/src/actions/publish.ts +++ b/src/actions/publish.ts @@ -274,11 +274,6 @@ function unwrapEncodedProfile(encodingResult: ProfileEncodingResult): Blob { return encodingResult.profileData; } -export type InflightProfileEncoding = { - sanitizedProfile: Profile; - encodingPromise: Promise; -}; - /** * Kick off "encoding" of the sanitized profile. Specifically this means: * - Compute the sanitized profile @@ -287,7 +282,7 @@ export type InflightProfileEncoding = { * * The asynchronous compression can take a few seconds, so we kick it off * immediately when the download or share panel is opened. The in-flight - * compression is tracked in the redux state, so opening the other panel or + * compression is tracked in the redux state, so reopening the panel or * pressing Upload reuses it (for the same sanitized profile) instead of * compressing again. The returned promise lets the caller wait on the * compressed result. @@ -296,8 +291,8 @@ export type InflightProfileEncoding = { */ export function encodeSanitizedProfile( mode: SharingMode -): ThunkAction { - return (dispatch, getState): InflightProfileEncoding => { +): ThunkAction> { + return (dispatch, getState): Promise => { const state = getState(); const sanitizedProfile = getSanitizedProfile(state, mode).profile; @@ -307,27 +302,21 @@ export function encodeSanitizedProfile( encodingState.sanitizedProfile === sanitizedProfile ) { // A compression for this profile is already in-flight; reuse it. - return { - sanitizedProfile, - encodingPromise: encodingState.encodingPromise, - }; + return encodingState.encodingPromise; } if ( encodingState.phase === 'DONE' && encodingState.sanitizedProfile === sanitizedProfile ) { // We already have an encoded version of this profile in our state! Use it. - return { - sanitizedProfile, - encodingPromise: Promise.resolve({ - type: 'SUCCESS', - profileData: encodingState.profileData, - }), - }; + return Promise.resolve({ + type: 'SUCCESS', + profileData: encodingState.profileData, + }); } // Kick off a new encoding for this profile. Don't await the promise, - // just return it as part of the InflightProfileEncoding. + // just hand it back to the caller. const encodingPromise: Promise = (async function () { // Yield so the ENCODING phase (dispatched below) is in the store before a // synchronous failure here could dispatch FAILED, which the reducer drops. @@ -351,7 +340,7 @@ export function encodeSanitizedProfile( dispatch( sanitizedProfileEncodingStarted(mode, sanitizedProfile, encodingPromise) ); - return { sanitizedProfile, encodingPromise }; + return encodingPromise; }; } @@ -402,8 +391,7 @@ export function attemptToPublish(): ThunkAction> { prePublishedState, 'upload' ); - const profileEncoding = dispatch(encodeSanitizedProfile('upload')); - const encodingResult = await profileEncoding.encodingPromise; + const encodingResult = await dispatch(encodeSanitizedProfile('upload')); // The previous line was async, check to make sure that this request is still valid. // The upload could have been aborted while we were compressing the data. diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index e8a235518c..99649878fc 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -190,8 +190,8 @@ describe('encodeSanitizedProfile', function () { const first = dispatch(encodeSanitizedProfile('upload')); const second = dispatch(encodeSanitizedProfile('upload')); - expect(second.encodingPromise).toBe(first.encodingPromise); - await first.encodingPromise; + expect(second).toBe(first); + await first; }); it('keeps a separate encoding per mode, so alternating panel opens each reuse their own', async function () { @@ -202,17 +202,13 @@ describe('encodeSanitizedProfile', function () { // encoding... const download = dispatch(encodeSanitizedProfile('download')); const upload = dispatch(encodeSanitizedProfile('upload')); - expect(upload.encodingPromise).not.toBe(download.encodingPromise); + expect(upload).not.toBe(download); // ...but reopening a panel must not start a third and fourth encoding. - expect(dispatch(encodeSanitizedProfile('download')).encodingPromise).toBe( - download.encodingPromise - ); - expect(dispatch(encodeSanitizedProfile('upload')).encodingPromise).toBe( - upload.encodingPromise - ); + expect(dispatch(encodeSanitizedProfile('download'))).toBe(download); + expect(dispatch(encodeSanitizedProfile('upload'))).toBe(upload); - await Promise.all([download.encodingPromise, upload.encodingPromise]); + await Promise.all([download, upload]); }); it('never exposes a mode the blob that was encoded for the other mode', async function () { @@ -229,7 +225,7 @@ describe('encodeSanitizedProfile', function () { // Opt into URLs for uploading only, and let that encoding finish. dispatch(updateSharingOption('upload', 'includeUrls', true)); - await dispatch(encodeSanitizedProfile('upload')).encodingPromise; + await dispatch(encodeSanitizedProfile('upload')); expect(getSanitizedProfileEncodingState(getState(), 'upload').phase).toBe( 'DONE' ); @@ -242,7 +238,7 @@ describe('encodeSanitizedProfile', function () { expect(forDownload.phase).toBe('INITIAL'); // And once download has encoded too, each mode keeps its own blob. - await dispatch(encodeSanitizedProfile('download')).encodingPromise; + await dispatch(encodeSanitizedProfile('download')); const encodedBlob = (mode: SharingMode) => { const encodingState = getSanitizedProfileEncodingState(getState(), mode); if (encodingState.phase !== 'DONE') { From 6e6c878908aa49302e8c4a34d2c2b4a92c732bd1 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 13 Aug 2026 11:18:04 -0400 Subject: [PATCH 3/9] Publish panel: prevent the default form submission. --- src/components/app/MenuButtons/Publish.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 3cce582090..5a2be0d4fd 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -123,7 +123,16 @@ class PublishPanelImpl extends React.PureComponent { ); } - _onSubmit = () => { + _onSubmit = (event: React.FormEvent) => { + // Without this, submitting the form navigates to the current URL and + // reloads the whole app. Today that is masked by attemptToPublish + // dispatching synchronously, which unmounts the form before the browser + // gets to the default action -- don't rely on that. + event.preventDefault(); + if (this.props.mode === 'download') { + // The download panel has no submit button; there is nothing to publish. + return; + } this.props.attemptToPublish(); }; @@ -166,7 +175,7 @@ class PublishPanelImpl extends React.PureComponent {

{title}

From ce51931ff39ca4c91a763614ef2a52ba8324194c Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 13 Aug 2026 11:28:17 -0400 Subject: [PATCH 4/9] Publish panel: only lock the sharing options of the mode that is uploading. --- src/components/app/MenuButtons/Publish.tsx | 8 ++++++-- src/test/components/MenuButtons.test.tsx | 7 ++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 5a2be0d4fd..10e3a9a516 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -102,9 +102,13 @@ class PublishPanelImpl extends React.PureComponent { labelL10nId: string, additionalContent?: React.ReactNode ) { - const { checkedSharingOptions, uploadPhase } = this.props; + const { checkedSharingOptions, uploadPhase, mode } = this.props; + // Only the panel whose upload is in flight needs to freeze its options. + // The two modes have independent options, so an upload started from Share + // is no reason to stop someone adjusting what they save to disk. const isUploading = - uploadPhase === 'uploading' || uploadPhase === 'compressing'; + mode === 'upload' && + (uploadPhase === 'uploading' || uploadPhase === 'compressing'); return (

- {sanitizedProfileEncodingState.phase === 'ERROR' ? ( + {sanitizedProfileEncodingStates.jslb.phase === 'ERROR' ? (
@@ -281,26 +317,22 @@ class PublishPanelImpl extends React.PureComponent { ) : null}
{isDownload ? ( - + this._renderDownloadButton(true) ) : (
- + {this._renderDownloadButton(false)}
+
@@ -2336,24 +2350,38 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for
- - - Download - - - ( - 1.58 kB - ) - - + + Download + + + ( + 1.58 kB + ) + + +
+
+
@@ -2450,24 +2478,38 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for
- - - Download - - - ( - 1.58 kB - ) - - + + Download + + + ( + 1.58 kB + ) + + +
+
+
diff --git a/src/test/store/publish.test.ts b/src/test/store/publish.test.ts index 99649878fc..90ebaaaab4 100644 --- a/src/test/store/publish.test.ts +++ b/src/test/store/publish.test.ts @@ -187,8 +187,8 @@ describe('encodeSanitizedProfile', function () { const { profile } = getProfileFromTextSamples('A'); const { dispatch } = storeWithProfile(profile); - const first = dispatch(encodeSanitizedProfile('upload')); - const second = dispatch(encodeSanitizedProfile('upload')); + const first = dispatch(encodeSanitizedProfile('upload', 'jslb')); + const second = dispatch(encodeSanitizedProfile('upload', 'jslb')); expect(second).toBe(first); await first; @@ -200,13 +200,13 @@ describe('encodeSanitizedProfile', function () { // The two modes sanitize to different profiles, so they each need their own // encoding... - const download = dispatch(encodeSanitizedProfile('download')); - const upload = dispatch(encodeSanitizedProfile('upload')); + const download = dispatch(encodeSanitizedProfile('download', 'jslb')); + const upload = dispatch(encodeSanitizedProfile('upload', 'jslb')); expect(upload).not.toBe(download); // ...but reopening a panel must not start a third and fourth encoding. - expect(dispatch(encodeSanitizedProfile('download'))).toBe(download); - expect(dispatch(encodeSanitizedProfile('upload'))).toBe(upload); + expect(dispatch(encodeSanitizedProfile('download', 'jslb'))).toBe(download); + expect(dispatch(encodeSanitizedProfile('upload', 'jslb'))).toBe(upload); await Promise.all([download, upload]); }); @@ -225,22 +225,27 @@ describe('encodeSanitizedProfile', function () { // Opt into URLs for uploading only, and let that encoding finish. dispatch(updateSharingOption('upload', 'includeUrls', true)); - await dispatch(encodeSanitizedProfile('upload')); - expect(getSanitizedProfileEncodingState(getState(), 'upload').phase).toBe( - 'DONE' - ); + await dispatch(encodeSanitizedProfile('upload', 'jslb')); + expect( + getSanitizedProfileEncodingState(getState(), 'upload', 'jslb').phase + ).toBe('DONE'); // The download panel, whose options still strip URLs, must not see it. const forDownload = getSanitizedProfileEncodingState( getState(), - 'download' + 'download', + 'jslb' ); expect(forDownload.phase).toBe('INITIAL'); // And once download has encoded too, each mode keeps its own blob. - await dispatch(encodeSanitizedProfile('download')); + await dispatch(encodeSanitizedProfile('download', 'jslb')); const encodedBlob = (mode: SharingMode) => { - const encodingState = getSanitizedProfileEncodingState(getState(), mode); + const encodingState = getSanitizedProfileEncodingState( + getState(), + mode, + 'jslb' + ); if (encodingState.phase !== 'DONE') { throw new Error( `Expected the ${mode} encoding to be DONE, but it is ${encodingState.phase}.` diff --git a/src/types/actions.ts b/src/types/actions.ts index 1a6ec0a82c..3cfb38fd95 100644 --- a/src/types/actions.ts +++ b/src/types/actions.ts @@ -46,6 +46,7 @@ import type { TableViewOptions, DecodedInstruction, ProfileEncodingResult, + PublishProfileFormat, } from './state'; import type { CssPixels, StartEndRange, Milliseconds } from './units'; import type { BrowserConnectionStatus } from '../app-logic/browser-connection'; @@ -657,18 +658,21 @@ type PublishAction = | { readonly type: 'SANITIZED_PROFILE_ENCODING_STARTED'; readonly mode: SharingMode; + readonly format: PublishProfileFormat; readonly sanitizedProfile: Profile; readonly encodingPromise: Promise; } | { readonly type: 'SANITIZED_PROFILE_ENCODING_COMPLETED'; readonly mode: SharingMode; + readonly format: PublishProfileFormat; readonly sanitizedProfile: Profile; readonly profileData: Blob; } | { readonly type: 'SANITIZED_PROFILE_ENCODING_FAILED'; readonly mode: SharingMode; + readonly format: PublishProfileFormat; readonly sanitizedProfile: Profile; readonly error: Error; } diff --git a/src/types/state.ts b/src/types/state.ts index e01956a84c..0f4d1faf1d 100644 --- a/src/types/state.ts +++ b/src/types/state.ts @@ -237,15 +237,21 @@ export type SanitizedProfileEncodingState = } | { phase: 'ERROR'; sanitizedProfile: Profile; error: Error }; +// The upload path always uses 'jslb'; 'json' is only used for the "Download +// as JSON" option in the publish panel's split download button. +export type PublishProfileFormat = 'jslb' | 'json'; + export type PublishState = { readonly checkedSharingOptions: Record; - // One encoding slot per sharing mode. The two modes have independent - // sharing options, so they sanitize to different profiles and must not - // share a slot: otherwise each mode's encoding clobbers the other's, and a - // panel could offer a blob that was encoded for the *other* mode's options. + // One encoding slot per (sharing mode, format) pair. The two modes have + // independent sharing options, so they sanitize to different profiles and + // must not share a slot: otherwise each mode's encoding clobbers the + // other's, and a panel could offer a blob that was encoded for the *other* + // mode's options. The formats are independent for the same reason: each is + // a separate serialization, offered by a different control. readonly sanitizedProfileEncodingStates: Record< SharingMode, - SanitizedProfileEncodingState + Record >; readonly upload: UploadState; readonly isHidingStaleProfile: boolean; From a3ee218d1ea6b10ffc6a9f5749fd2e6bdcfaa656 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 15 Jul 2026 11:50:48 -0400 Subject: [PATCH 7/9] Publish panel: use react-contextmenu-style dropdown for download formats. Switch the split download button's dropdown from ButtonWithPanel/ArrowPanel to a react-contextmenu menu (the same style used next to the marker chart's search field). The toggle is a plain photon button with a CSS-drawn caret; showMenu opens the menu at the button's bottom-right. --- src/components/app/MenuButtons/Publish.css | 42 ++-- src/components/app/MenuButtons/Publish.tsx | 209 +++++++++++++----- .../__snapshots__/MenuButtons.test.tsx.snap | 96 +++++--- 3 files changed, 233 insertions(+), 114 deletions(-) diff --git a/src/components/app/MenuButtons/Publish.css b/src/components/app/MenuButtons/Publish.css index f403e1a9df..05a1ab70c4 100644 --- a/src/components/app/MenuButtons/Publish.css +++ b/src/components/app/MenuButtons/Publish.css @@ -183,13 +183,8 @@ border-top-right-radius: 0; } -.publishPanelSplitButtonToggle { - /* Override the flex-column from .buttonWithPanel — we want the button - * baseline to line up with the primary button, not stack. */ - flex-flow: row nowrap; -} - .publishPanelSplitButtonToggleButton { + position: relative; width: 24px; min-width: 0; height: 32px; @@ -200,31 +195,20 @@ cursor: default; } +/* Down-caret drawn with borders, matching the marker-chart filter/copy + * buttons' style. */ .publishPanelSplitButtonToggleButton::after { - content: '▾'; - font-size: 10px; - line-height: 1; -} - -.publishPanelSplitButtonPanel { - --width: 220px; -} - -.publishPanelSplitButtonMenuItem { - display: inline-block; + position: absolute; + top: 50%; + left: 50%; + width: 0; + height: 0; + border-top: 5px solid; + border-right: 4px solid transparent; + border-left: 4px solid transparent; color: var(--clickable-foreground-color); - cursor: default; - text-decoration: none; -} - -.publishPanelSplitButtonMenuItem:not( - .publishPanelSplitButtonMenuItemDisabled -):hover { - text-decoration: underline; -} - -.publishPanelSplitButtonMenuItemDisabled { - opacity: 0.6; + content: ''; + transform: translate(-50%, -25%); } .menuButtonsDownloadSize { diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 78c78c5a35..0a286d2389 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -29,7 +29,8 @@ import { getSanitizedProfileEncodingStates, } from 'firefox-profiler/selectors/publish'; import { BlobUrlLink } from 'firefox-profiler/components/shared/BlobUrlLink'; -import { ButtonWithPanel } from 'firefox-profiler/components/shared/ButtonWithPanel'; +import { ContextMenu } from 'firefox-profiler/components/shared/ContextMenu'; +import { MenuItem, showMenu } from '@firefox-devtools/react-contextmenu'; import { assertExhaustiveCheck } from 'firefox-profiler/utils/types'; import prettyBytes from 'firefox-profiler/utils/pretty-bytes'; @@ -482,6 +483,8 @@ export const PublishPanel = explicitConnect< component: PublishPanelImpl, }); +const DOWNLOAD_FORMAT_MENU_ID = 'PublishDownloadFormatContextMenu'; + type DownloadSplitButtonProps = { readonly primary: boolean; readonly jslbEncodingState: SanitizedProfileEncodingState; @@ -491,11 +494,63 @@ type DownloadSplitButtonProps = { readonly onJsonDropdownOpen: () => void; }; +type DownloadSplitButtonState = { + readonly isMenuVisible: boolean; + // react-contextmenu hides the menu on mousedown even if it's already + // visible. We track visibility at mousedown time so that clicking the + // toggle button while the menu is open results in a net "close" rather + // than a hide-then-reshow. + readonly wasMenuVisibleOnMouseDown: boolean; +}; + /** * Split button: the main body downloads the profile as JSLB, and the arrow * on the right opens a dropdown menu with alternate download formats. + * The dropdown uses the same react-contextmenu-based menu style as the + * marker chart's filter/copy buttons. */ -class DownloadSplitButton extends React.PureComponent { +class DownloadSplitButton extends React.PureComponent< + DownloadSplitButtonProps, + DownloadSplitButtonState +> { + override state: DownloadSplitButtonState = { + isMenuVisible: false, + wasMenuVisibleOnMouseDown: false, + }; + + _onToggleClick = (event: React.MouseEvent) => { + if (this.state.wasMenuVisibleOnMouseDown) { + // The menu was already open on mousedown, so react-contextmenu has + // already hidden it. Don't reopen it. + return; + } + + const rect = event.currentTarget.getBoundingClientRect(); + // The menu is right-aligned to the toggle button since it sits near the + // right edge of the publish panel. + showMenu({ + data: null, + id: DOWNLOAD_FORMAT_MENU_ID, + position: { x: rect.right, y: rect.bottom }, + target: event.target, + }); + }; + + _onToggleMouseDown = () => { + this.setState((state) => ({ + wasMenuVisibleOnMouseDown: state.isMenuVisible, + })); + }; + + _onMenuShow = () => { + this.setState({ isMenuVisible: true }); + this.props.onJsonDropdownOpen(); + }; + + _onMenuHide = () => { + this.setState({ isMenuVisible: false }); + }; + override render() { const { primary, @@ -503,8 +558,8 @@ class DownloadSplitButton extends React.PureComponent jsonEncodingState, jslbDownloadFileName, jsonDownloadFileName, - onJsonDropdownOpen, } = this.props; + const { isMenuVisible } = this.state; return (
@@ -513,19 +568,27 @@ class DownloadSplitButton extends React.PureComponent encodingState={jslbEncodingState} downloadFileName={jslbDownloadFileName} /> - - } +
); @@ -592,68 +655,98 @@ class DownloadPrimary extends React.PureComponent { } } -type DownloadFormatMenuProps = { +type DownloadFormatContextMenuProps = { + readonly menuId: string; + readonly onShow: () => void; + readonly onHide: () => void; readonly jsonEncodingState: SanitizedProfileEncodingState; readonly jsonDownloadFileName: string; }; /** - * Content of the dropdown panel: alternative download formats. + * Trigger a file download for a Blob without needing a persistent + * BlobUrlLink anchor. Used from a react-contextmenu MenuItem's onClick, + * where we can't wrap the item in an . */ -class DownloadFormatMenu extends React.PureComponent { - override render() { +function _triggerBlobDownload(blob: Blob, downloadFileName: string): void { + const url = URL.createObjectURL(blob); + const anchor = document.createElement('a'); + anchor.href = url; + anchor.download = downloadFileName; + anchor.style.display = 'none'; + document.body.appendChild(anchor); + anchor.click(); + document.body.removeChild(anchor); + // Revoke asynchronously so Safari has a chance to start the download. + setTimeout(() => URL.revokeObjectURL(url), 5000); +} + +/** + * The dropdown menu for alternative download formats, rendered as a + * react-contextmenu (the same menu style used next to the marker chart + * search field). + */ +class DownloadFormatContextMenu extends React.PureComponent { + _onDownloadJson = () => { const { jsonEncodingState, jsonDownloadFileName } = this.props; - const itemClassName = 'publishPanelSplitButtonMenuItem'; + if (jsonEncodingState.phase !== 'DONE') { + return; + } + _triggerBlobDownload( + jsonEncodingState.profileData, + `${jsonDownloadFileName}.gz` + ); + }; + _renderJsonMenuItemLabel() { + const { jsonEncodingState } = this.props; switch (jsonEncodingState.phase) { - case 'DONE': { - const { profileData } = jsonEncodingState; + case 'DONE': return ( - + <> Download as JSON - {' '} - - ({prettyBytes(profileData.size)}) - - + + {` (${prettyBytes(jsonEncodingState.profileData.size)})`} + ); - } - case 'ERROR': { + case 'ERROR': return ( - - - Error preparing JSON download - - + + Error preparing JSON download + ); - } case 'INITIAL': - case 'ENCODING': { + case 'ENCODING': return ( - - - Compressing… - - + + Compressing… + ); - } default: throw assertExhaustiveCheck(jsonEncodingState); } } + + override render() { + const { menuId, onShow, onHide, jsonEncodingState } = this.props; + const jsonReady = jsonEncodingState.phase === 'DONE'; + + return ( + + + {this._renderJsonMenuItemLabel()} + + + ); + } } diff --git a/src/test/components/__snapshots__/MenuButtons.test.tsx.snap b/src/test/components/__snapshots__/MenuButtons.test.tsx.snap index 91d2fd983f..63dfb0a8dd 100644 --- a/src/test/components/__snapshots__/MenuButtons.test.tsx.snap +++ b/src/test/components/__snapshots__/MenuButtons.test.tsx.snap @@ -2238,15 +2238,29 @@ exports[`app/MenuButtons matches the snapshot for the menu buttons and ) -
-
@@ -2371,15 +2385,29 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for ) -
-
@@ -2499,15 +2527,29 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for ) -
-
From 572308d0aa0ac1d36e0aa34d036182732e311c9f Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 15 Jul 2026 11:55:06 -0400 Subject: [PATCH 8/9] Publish panel: right-anchor the download formats dropdown menu. showMenu positions the menu's top-left at the given coords. Subtracting a fixed menu width from the toggle button's right edge right-anchors the menu so its top-right lines up with the button's bottom-right, and it grows leftwards into the panel instead of off toward the right edge. --- src/components/app/MenuButtons/Publish.css | 7 +++++++ src/components/app/MenuButtons/Publish.tsx | 13 ++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/components/app/MenuButtons/Publish.css b/src/components/app/MenuButtons/Publish.css index 05a1ab70c4..1162af6384 100644 --- a/src/components/app/MenuButtons/Publish.css +++ b/src/components/app/MenuButtons/Publish.css @@ -211,6 +211,13 @@ transform: translate(-50%, -25%); } +/* Pin the download-format menu to a fixed width. The JS side subtracts this + * width from the toggle button's right edge to right-anchor the menu; the + * two must stay in sync. */ +.publishPanelDownloadFormatContextMenu { + width: 220px; +} + .menuButtonsDownloadSize { display: inline-block; margin: 0 4px; diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index 0a286d2389..a51fb546df 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -484,6 +484,10 @@ export const PublishPanel = explicitConnect< }); const DOWNLOAD_FORMAT_MENU_ID = 'PublishDownloadFormatContextMenu'; +// react-contextmenu positions the menu with its top-left at `position`. To +// right-anchor to the toggle button we subtract an estimated menu width; the +// actual width is pinned to this value via CSS. +const DOWNLOAD_FORMAT_MENU_WIDTH_PX = 220; type DownloadSplitButtonProps = { readonly primary: boolean; @@ -526,12 +530,15 @@ class DownloadSplitButton extends React.PureComponent< } const rect = event.currentTarget.getBoundingClientRect(); - // The menu is right-aligned to the toggle button since it sits near the - // right edge of the publish panel. + // Right-anchor the menu: its top-right sits at the toggle button's + // bottom-right, so it grows leftwards into the panel. showMenu({ data: null, id: DOWNLOAD_FORMAT_MENU_ID, - position: { x: rect.right, y: rect.bottom }, + position: { + x: rect.right - DOWNLOAD_FORMAT_MENU_WIDTH_PX, + y: rect.bottom, + }, target: event.target, }); }; From 0d4b417072fa305214f459aeb21094c5203d84d4 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Wed, 15 Jul 2026 12:07:09 -0400 Subject: [PATCH 9/9] Publish panel: defer JSON encoding until after the dropdown paints. encodeSanitizedProfile's promise body runs its serializer synchronously up to the first await, so kicking it off from the menu's onShow blocked the menu's first paint on large profiles. setTimeout(0) yields to the browser so the menu appears immediately. --- src/components/app/MenuButtons/Publish.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx index a51fb546df..ff9af1f56a 100644 --- a/src/components/app/MenuButtons/Publish.tsx +++ b/src/components/app/MenuButtons/Publish.tsx @@ -551,7 +551,10 @@ class DownloadSplitButton extends React.PureComponent< _onMenuShow = () => { this.setState({ isMenuVisible: true }); - this.props.onJsonDropdownOpen(); + // Defer the JSON encoding so the menu can paint first. The serializer + // is synchronous and can take a while on large profiles; running it on + // the current task would delay the menu's first paint. + setTimeout(this.props.onJsonDropdownOpen, 0); }; _onMenuHide = () => {