Repository navigation
fix: handle cancelled and Save As DevTools saves correctly - #55484
Merged
MarshallOfSound merged 7 commits intoOct 30, 2026
Conversation
WebContents::DevToolsSaveToFile looked up the path already chosen for a URL but discarded the result, so every DevTools "Save" showed the save dialog again. Assign the lookup result so a repeat save writes to the stored path. Let the mock xdg-desktop-portal FileChooser queue a successful response and add a spec that saves the same URL twice and expects one SaveFile request. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
…lace the path Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
…e As replace the path" Moving to a separate PR. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
…lace the path Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkNiiBq2fQEsXWusxpm Co-authored-by: Claude <[email protected]>
3 tasks done
trop
Bot
added
44-x-y
backport
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
shell/browser/api/electron_api_web_contents.cc— DevTools users still get a save dialog on every plain "Save", even though the PR notes say this backport fixes it. The new reuse spec will also fail.base::FindOrNull(saved_files_, url);at line 4900 throws its result away, sopathstays nullptr and the stored path is never used. Fix: assign the lookup,path = base::FindOrNull(saved_files_, url);. This is the #55431 hunk this backport left out. Keep the!save_asguard so "Save As" still prompts.Why this was flagged
In DevToolsSaveToFile (electron_api_web_contents.cc:4898-4899),
if (!save_as) base::FindOrNull(saved_files_, url);throws the returned pointer away.paththerefore stays nullptr and line 4901 always opens ShowSaveDialogSync, even for a URL already in saved_files_. The diff only changes the dialog-result branch and leaves this line exactly as the base has it. The PR notes promise a fix for "DevTools 'Save' prompting for a location every time", but that does not happen after merge. The new spec 'reuses the saved path for a repeat save of the same URL' (spec/api-web-contents.spec.ts) fails as well: the second save sends a second SaveFile request, the mock's queue is empty so it answers cancelled, the file keeps 'first', andwaitUntil(() => readFile(target) === 'second')times out.Verification: At shell/browser/api/electron_api_web_contents.cc:4900 the code is
if (!save_as) base::FindOrNull(saved_files_, url);. The pointer the lookup returns is thrown away, sopathis still nullptr when line 4902 checksif (path == nullptr), and file_dialog::ShowSaveDialogSync (line 4909) opens on every save. The diff leaves line 4900 as the base has it.
The one-line change from #55431 that assigns the base::FindOrNull lookup result to `path` in WebContents::DevToolsSaveToFile was dropped from that PR's final diff after a Chromium roll on main fixed the same line. This branch does not have that roll, so restore the assignment here; without it every DevTools "Save" shows the save dialog again. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KX1hkLV75VR3EtD7cwybpM
Contributor
There was a problem hiding this comment.
LGTM. With the latest commit, the stored-path lookup result is assigned again, so this backport behaves as described on 44-x-y.
What was reviewed:
DevToolsSaveToFilein shell/browser/api/electron_api_web_contents.cc: a plain save now reuses the stored path. An empty or nullopt dialog result reachescanceledSaveURLand stores nothing. "Save as" replaces the stored path throughinsert_or_assign.- The mock portal's
QueueResponse/ClearQueuedResponses: they use the same@ dbus.service.method(MOCK_IFACE, ...)pattern as other dbusmock templates, answer requests oldest-first, and fall back to a cancel when nothing is queued. - The new spec suite is tagged
serialand Linux-gated. Every test clears queued responses withdefer, and assertions run in the test body rather than in listeners.dbus-nativeis already a spec dependency.
Extended reasoning...
The change touches one C++ function (about 9 lines in DevTools file saving on the browser side), the Python mock of the xdg-desktop-portal FileChooser, and adds one spec suite. The saved path comes from the native save dialog, not from renderer input, so the change opens no new security-sensitive surface. Since the earlier review, a follow-up commit restored the missing path = base::FindOrNull(...) assignment. Without that line, the "reuse saved path" fix and its test would not hold on this branch. The final C++ logic is small, clearly correct, and covered by three new Linux tests, and no reviewer has an open objection.
dsanders11
approved these changes
Oct 29, 2026
MarshallOfSound
approved these changes
Oct 30, 2026
MarshallOfSound
enabled auto-merge (squash)
October 30, 2026 06:24
MarshallOfSound
deleted the
trop/44-x-y-bp-fix-handle-cancelled-and-save-as-devtools-saves-correctly-1793307783065
branch
October 30, 2026 06:24
|
Release Notes Persisted
|
renovate Bot
added a commit
to cigaleapp/cigale
that referenced
this pull request
Nov 8, 2026
##### [v44.10.1](https://github.com/electron/electron/releases/tag/v44.10.1) ### Release Notes for v44.10.1 #### Fixes - Fixed DevTools "Save" prompting for a location every time instead of reusing the previously chosen file. [#55484](electron/electron#55484) - Fixed DevTools "Save As" using the old file and Linux cancels treated as saved. - Fixed `webContents.adjustSelection()` ignoring the `end` option. [#55461](electron/electron#55461) <sup>(Also in [45](electron/electron#55462), [46](https://github.com/electron/electron/pull/55463))</sup> #### Performance Improvements - Improved performance of `net.fetch()` requests without a body. [#55445](electron/electron#55445) <sup>(Also in [43](electron/electron#55447), [45](electron/electron#55446), [46](https://github.com/electron/electron/pull/55444))</sup> - Improved performance of repeated `net.fetch()` calls for `file:` URLs. [#55458](electron/electron#55458) <sup>(Also in [45](electron/electron#55459), [46](https://github.com/electron/electron/pull/55457))</sup> #### Other Changes - Backported fixes from upstream Chromium, Dawn, Skia and V8. [#55492](electron/electron#55492) ##### [v44.10.0](https://github.com/electron/electron/releases/tag/v44.10.0) ### Release Notes for v44.10.0 #### Features - Added filtering options to `session.registerPreloadScript` - `matches`, `excludeMatches`, `includeGlobs`, `excludeGlobs`, `matchOriginAsFallback`. [#55182](electron/electron#55182) <sup>(Also in [45](https://github.com/electron/electron/pull/55181))</sup> #### Fixes - Fixed a crash on Linux when a D-Bus method call was made on a disconnected bus. [#55290](electron/electron#55290) <sup>(Also in [45](electron/electron#55292), [46](https://github.com/electron/electron/pull/55289))</sup> - Fixed a main process crash when `onSendHeaders` observed Blob uploads. [#55168](electron/electron#55168) <sup>(Also in [45](https://github.com/electron/electron/pull/55169))</sup> - Fixed `app.getLoginItemSettings()` not finding login items on Windows when the executable path contains spaces. [#55207](electron/electron#55207) <sup>(Also in [45](https://github.com/electron/electron/pull/54419))</sup> - Fixed Europe/London and Europe/Dublin time zone names showing as "GMT+00:00" in winter by updating ICU to tzdata 2026e. [#55335](electron/electron#55335) - Fixed File System Access blocklist gaps for `.git/hooks` writes and hypervisor UNC shares. [#55330](electron/electron#55330) - Fixed handling of malformed asar archives whose size fields do not match the file length. [#55176](electron/electron#55176) <sup>(Also in [43](electron/electron#55179), [45](https://github.com/electron/electron/pull/55175))</sup> - Fixed inconsistent handling of asar archive paths between the main process and child processes. [#55183](electron/electron#55183) <sup>(Also in [43](electron/electron#55185), [45](https://github.com/electron/electron/pull/55184))</sup> - Fixed `Menu.popup()` on macOS sometimes not showing a menu after another closed. [#55325](electron/electron#55325) <sup>(Also in [45](electron/electron#55326), [46](https://github.com/electron/electron/pull/55324))</sup> - Fixed MV3 extension events not arriving to background service workers. [#55152](electron/electron#55152) <sup>(Also in [43](electron/electron#55153), [45](https://github.com/electron/electron/pull/55154))</sup> - Fixed paste hanging indefinitely on Linux X11 until the app was restarted. [#55327](electron/electron#55327) - Fixed `protocol.registerSource` schemes not applying the same fetch rules as `protocol.handle` schemes. [#55177](electron/electron#55177) <sup>(Also in [45](https://github.com/electron/electron/pull/55178))</sup> - Fixed `utilityProcess` not emitting an `exit` event when the app quit while the process was still launching. [#55273](electron/electron#55273) <sup>(Also in [43](electron/electron#55272), [45](electron/electron#55274), [46](https://github.com/electron/electron/pull/55264))</sup> #### Other Changes - Updated Chromium to 152.0.7977.169. [#55033](electron/electron#55033) - Updated Node.js to v24.22.1. [#55162](electron/electron#55162) ##### [v44.9.3](https://github.com/electron/electron/releases/tag/v44.9.3) ### Release Notes for v44.9.3 #### Other Changes - Backported fixes from upstream ANGLE, Chromium, PDFium, Skia and V8. [#55149](electron/electron#55149) ##### [v44.9.2](https://github.com/electron/electron/releases/tag/v44.9.2) ### Release Notes for v44.9.2 ##### [v44.9.1](https://github.com/electron/electron/releases/tag/v44.9.1) ### Release Notes for v44.9.1 > \[!NOTE] > This release contains the fix for <GHSA-jr5w-w6x2-84p9>, the mitigation for compromised eMudhra intermediate certificates ##### [v44.9.0](https://github.com/electron/electron/releases/tag/v44.9.0) ### Release Notes for v44.9.0 #### Features - Added app shims on macOS to show windows under another app's Dock icon. [#55073](electron/electron#55073) <sup>(Also in [45](https://github.com/electron/electron/pull/55072))</sup> #### Fixes - Fixed link-opened windows not using the opener's session and `webPreferences`. [#55048](electron/electron#55048) <sup>(Also in [42](electron/electron#55050), [43](electron/electron#55049), [45](https://github.com/electron/electron/pull/55051))</sup> - Fixed `setCertificateVerifyProc` occasionally not being applied to a request issued concurrently with the first request to a host. [#55046](electron/electron#55046) <sup>(Also in [42](electron/electron#55054), [43](electron/electron#55047), [45](https://github.com/electron/electron/pull/55045))</sup> ##### [v44.8.0](https://github.com/electron/electron/releases/tag/v44.8.0) ### Release Notes for v44.8.0 #### Features - Added support for `Notification` action buttons on Linux. [#54890](electron/electron#54890) <sup>(Also in [45](https://github.com/electron/electron/pull/54889))</sup> #### Fixes - Fixed mouse cursor missing over native dialogs in certain scenarios on Windows. [#54987](electron/electron#54987) - Fixed `net.fetch` and `session.fetch` failing permanently with `net::ERR_FAILED` after the Network Service process restarts. [#54909](electron/electron#54909) <sup>(Also in [42](electron/electron#54925), [43](electron/electron#54908), [45](https://github.com/electron/electron/pull/54910))</sup> - Fixed `utilityProcess` dropping messages posted with `process.parentPort.postMessage()` right before the process exits. [#54960](electron/electron#54960) <sup>(Also in [42](electron/electron#55000), [43](electron/electron#54962), [45](https://github.com/electron/electron/pull/54956))</sup> - Fixed `win.isFullScreen()` being stale in fullscreen events on Windows. [#55009](electron/electron#55009) <sup>(Also in [45](https://github.com/electron/electron/pull/55008))</sup> #### Other Changes - Backported fixes from upstream Chromium and V8. [#55034](electron/electron#55034) - Updated Chromium to 152.0.7977.167. [#54928](electron/electron#54928) - Updated Node.js to v24.22.0. [#55004](electron/electron#55004) ##### [v44.7.2](https://github.com/electron/electron/releases/tag/v44.7.2) ### Release Notes for v44.7.2 #### Fixes - Fixed an issue where clicks posted to a background app were dropped on macOS. [#54819](electron/electron#54819) <sup>(Also in [45](https://github.com/electron/electron/pull/54818))</sup> - Fixed interrupted macOS updates leaving a broken app behind. [#54844](electron/electron#54844) <sup>(Also in [45](https://github.com/electron/electron/pull/54843))</sup> #### Other Changes - Backported fixes from upstream ANGLE, Chromium, PDFium and V8. [#54898](electron/electron#54898) #### Documentation - Documentation changes: [#54880](electron/electron#54880) ##### [v44.7.1](https://github.com/electron/electron/releases/tag/v44.7.1) ### Release Notes for v44.7.1 #### Fixes - Fixed a crash when starting without an available display server on Linux. [#54794](electron/electron#54794) <sup>(Also in [45](https://github.com/electron/electron/pull/54781))</sup> - Fixed a Linux crash creating a window with a menu bar when GTK failed to load. [#54859](electron/electron#54859) <sup>(Also in [45](https://github.com/electron/electron/pull/54846))</sup> - Fixed a `WebContentsView` that was removed with `removeChildView()` and re-added with `addChildView()` staying hidden behind its sibling views on Linux. [#54798](electron/electron#54798) - Fixed asar archives in non-ASCII folders failing to load on Linux. [#54849](electron/electron#54849) <sup>(Also in [45](https://github.com/electron/electron/pull/54848))</sup> - Fixed renderer crashes during garbage collection inside ShadowRealms. [#54865](electron/electron#54865) <sup>(Also in [45](https://github.com/electron/electron/pull/54864))</sup> #### Other Changes - Refactored mouse forwarding on Windows to fix `setIgnoreMouseEvents(true, { forward: true })` related issues. [#54802](electron/electron#54802) <sup>(Also in [45](https://github.com/electron/electron/pull/54803))</sup> - Updated Chromium to 152.0.7977.149. [#54085](electron/electron#54085) ##### [v44.7.0](https://github.com/electron/electron/releases/tag/v44.7.0) ### Release Notes for v44.7.0 #### Features - The macOS `autoUpdater` now streams update downloads to disk, resumes an interrupted download on the next check instead of starting over, and applies a binary delta when the update feed offers one for the running version. [#54204](electron/electron#54204) #### Fixes - Fixed an uncatchable TypeError in `net.fetch` when receiving response headers with non-ASCII characters, and preserved multiple Set-Cookie headers. [#54672](electron/electron#54672) #### Other Changes - Backported fixes from upstream ANGLE, Chromium, Dawn, fontconfig, Skia, V8 and WebRTC. [#54666](electron/electron#54666) - Backported fixes from upstream ANGLE, Chromium, Skia, V8 and WebRTC. [#54657](electron/electron#54657) ##### [v44.6.0](https://github.com/electron/electron/releases/tag/v44.6.0) ### Release Notes for v44.6.0 #### Features - Added the experimental `protocol.registerSource()` to serve a custom scheme from directories on disk without a JavaScript handler. [#54522](electron/electron#54522) <sup>(Also in [45](https://github.com/electron/electron/pull/54521))</sup> #### Fixes - Fixed a crash on launch on macOS when the system refused to register the app for sleep/wake notifications. [#54596](electron/electron#54596) - Fixed a crash on `navigator.usb.requestDevice()` in in-memory sessions. [#54616](electron/electron#54616) <sup>(Also in [45](https://github.com/electron/electron/pull/54615))</sup> - Fixed `webview.findInPage()` hanging when the host page also has an iframe. [#54586](electron/electron#54586) <sup>(Also in [43](electron/electron#54584), [45](https://github.com/electron/electron/pull/54585))</sup> - Updated time zone data to tzdata 2026d, including British Columbia's move to permanent UTC-7. [#54623](electron/electron#54623) #### Documentation - Documentation changes: [#54589](electron/electron#54589)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #55431
Backport of #55434
See those PRs for details.
Notes: