1366061 - "Save" fails in Memory snapshot tool

Status: VERIFIED FIXED

(DevTools :: Memory, defect)

Description

[Jacob]

Attachment: Screenshot 2017-05-18 16.42.34.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170504105526

Steps to reproduce:

Open the console
Choose "Memory"
Take snapshot
Attempt to save snapshot


Actual results:

Nothing. If I double-click, the word "Save" is highlighted.


Expected results:

https://developer.mozilla.org/en-US/docs/Tools/Memory/Basic_operations#Saving_and_loading_snapshots says “You'll be prompted for a name and location, and the file will be saved with an .fxsnapshot extension.”

Reproduces in Firefox 53.0.2 (64-bit) macOS and in 53.0.2 (32- and 64-bit) Windows.

Probably related: https://bugzilla.mozilla.org/show_bug.cgi?id=1332107

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I can reproduce.

Comment 2

[Jacob]

Browser Console shows:

ThreadSafeDevToolsUtils.js:80

"@@redux/middleware/task#error threw an exception: Error: No valid mode specified for nsIFilePicker.
Stack: exports.openFilePicker@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/memory/utils.js:421:11
exports.pickFileAndExportSnapshot/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/memory/actions/io.js:20:28
_run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:273:3
asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:247:14
spawn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:161:12
task/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/redux/middleware/task.js:21:14
onSave@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/memory/app.js:215:33
onClick@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/memory/components/snapshot-list-item.js:88:22
invokeGuardedCallback@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:11386:12
executeDispatch@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:2893:5
executeDispatchesInOrder@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:2913:7
executeDispatchesAndRelease@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:2348:5
executeDispatchesAndReleaseTopLevel@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:2359:10
forEachAccumulated@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:19780:5
processEventQueue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:2535:7
runEventQueueInBatch@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:11455:3
handleTopLevel@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:11466:5
handleTopLevelImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:11544:5
perform@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:19035:13
batchedUpdates@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:10660:7
batchedUpdates@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:16668:3
dispatchEvent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/shared/vendor/react.js:11621:7
Line: 421, column: 11"

Comment 3

[Greg Tatum [:gregtatum]]

Attachment: Bug 1366061 - Fix memory tool file picker;

Review commit: https://reviewboard.mozilla.org/r/141168/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141168/

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8869592 [details]
Bug 1366061 - Fix memory tool file picker;

https://reviewboard.mozilla.org/r/141168/#review144750

Thanks for the fix!

Super nit-picky, but I think the commit message could be a bit more detailed / expressive. I don't think it stands on its own if someone else were to read the message and look at the diff.

Thanks again!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c91f590eb02
Fix memory tool file picker; r=fitzgen

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2c91f590eb02

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Do we need to uplift this?

Comment 8

[Greg Tatum [:gregtatum]]

Yes. It goes up all the way to release. Should I request uplifts all the way up?

Comment 9

[Greg Tatum [:gregtatum]]

Comment on attachment 8869592 [details]
Bug 1366061 - Fix memory tool file picker;

Approval Request Comment
[Feature/Bug causing the regression]:
Large code changes to make memory tool Eslint compliant. : Bug 1251728

[User impact if declined]:
Save button will not work in devtools memory tool.

[Is this code covered by automated tests?]:
The surrounding code is for exporting snapshots, but the privileged code where the regression was located is not.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes.
- Open up devtools.
- Go to memory panel.
- Take a snapshot
- Hit save on the snapshot in the list on the left
- It should allow you to save the memory snapshot.
Actual results: Nothing happens when you click the button.

[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
No

[Why is the change risky/not risky?]:
It's a JS change to a developer tool with low open rates.

[String changes made/needed]: None.

Comment 10

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 11

[Md. Majedul islam]

I have reproduced this bug with Nightly 55.0a1 (2017-05-18)  on Ubuntu 16.04, 64 bit!

The fix is now verified on Latest Nightly.

Build ID 	20170523100217
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170524]

Comment 12

[Brindusa Tot, Desktop QA]

Reproduced the bug with a Nightly 55.0a1 from 05/10/2017 on Windows 10 x64.

Verified as fixed on latest Nightly 55.0a1, build ID 20170524030204, on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.12.

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8869592 [details]
Bug 1366061 - Fix memory tool file picker;

Fix an issue related to memory snapshot tool and was verified. Beta54+. Should be in 54 beta 11.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6ff47bf39feb

Comment 15

[Emil Ghitta, QA [:emilghitta]]

I have reproduced the issue mentioned in comment 0 using Firefox 55.0a1 (Build Id:20170518030213).

This issue is verified fixed on Firefox 54.0 (Build Id:20170605134926) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 14.04 32bit