Export Console content to file
Categories
(DevTools :: Console, enhancement, P3)
Tracking
(firefox69 fixed)
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: jefry.reyes)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/plain
|
Details |
Bug 644412 adds the ability to save the content to the clipboard. We could have another context menu entry to save to a file. We may want to group those 2 entries into one and have a sub menu
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
Hi Nicolas. I thought I would ask for some feedback before I continue working on this, to make sure I'm going in the right path.
I want to leave showFilePicker general so we can use it in the style editor and not duplicate code that way. For the other parts there are some decisions that need to be made with regards of the extension of the file, whether the last file saved should be remembered so it is automatically selected a if the user wants to save the file a second time, the title of the file picker window, the default name if any of the file, etc.
Thanks!
Reporter | ||
Comment 2•6 years ago
|
||
Comment on attachment 9044582 [details] [diff] [review] Bug1517728.patch Review of attachment 9044582 [details] [diff] [review]: ----------------------------------------------------------------- Hello Jefry, Thanks for working on this! For now, I think we shouldn't copy everything the StyleEditor is doing, and we should tailor showFilePicker to the console need (we can modify it later, let's keep it simple for now). ::: devtools/client/webconsole/utils/context-menu.js @@ +203,5 @@ > + const webconsoleOutput = parentNode.querySelector(".webconsole-output"); > + const istream = converter.convertToInputStream(webconsoleOutput.textContent); > + DevToolsUtils.saveFileStream(returnFile, istream); > + } > + // TODO: Which filter is better than *? Which default name if any? Which path? The filter is fine I think (we want people to name it as they want). A default name like `console-export-${YYYY-MM-DD_hh-mm-ss}.txt` would be nice. ::: devtools/shared/DevToolsUtils.js @@ +813,5 @@ > + * Optional nsIFile or string representing the filename to auto-select. > + * @param nsIWindow parentWindow > + * Optional parent window. If null the parent window of the file picker > + * will be the window of the attached input element. > + * @param callback It would be more consistent with the rest of the codebase if the function would return a Promise (or be an async function). @@ +823,5 @@ > + * Optional name shown to the user that describes the file filter. > + * @param AString fileFilter > + * Optional the filter used to specify file type. > + */ > +// TODO: use this as the filepicker for the style editor. we should file a Bug to do it, and reference it from here. @@ +824,5 @@ > + * @param AString fileFilter > + * Optional the filter used to specify file type. > + */ > +// TODO: use this as the filepicker for the style editor. > +exports.showFilePicker = function(path, toSave, parentWindow, callback, since it's a new file, and we are only calling it from one place, I think we can remove some of those params. If we need them in the future, we'll update the function, but for now, let's only make it match what we need in the console. - `path` could be removed, since we pass `null` from the console (and it will simplify the code a lot) - `toSave` can be removed since we pass `true` from the console @@ +871,5 @@ > + } > + > + // TODO: getString is used to get the l10 string. Which string > + // would be appropiate? > + fp.init(parentWindow, fileFilterName + ".title", mode); I'm not sure what this is doing? Also, shouldn't we check that filterFilterName and fileFilter are defined before adding this filter?
Assignee | ||
Comment 3•6 years ago
|
||
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 5•6 years ago
|
||
Testing the filepicker is the most challenging part. I'm still stuck on it.
Assignee | ||
Comment 6•6 years ago
|
||
Hello Nicolas, so now I have managed to open the file picker in mochitest and save the file. My problem now is that when the file is saved, it appears it isn't flush immediately to disk so when I try to open it to verify that it has all of the output of the console, it gives me an error that it can't find the file.
Do you have an idea what could be causing this timing issue?
Reporter | ||
Comment 7•6 years ago
|
||
Hello Jefry.
Could you post your patch somewhere so I can give it a try and check what is going on? Thanks!
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
Hi Nicolas, I have attached the mochitest as I'm reworking it. You will find that in the exportAllToFile function, the issue is that the file is being read before it is written. How do I make sure it is flushed?
Reporter | ||
Comment 9•6 years ago
|
||
Hello Jefry, thanks for posting your patch, that was helpful!
So, I was able to get the text of the file with those 2 functions:
async function exportAllToFile(hud, message) {
const menuPopup = await openContextMenu(hud, message);
const exportMenu = menuPopup.querySelector("#console-menu-export");
const view = exportMenu.ownerDocument.defaultView;
EventUtils.synthesizeMouseAtCenter(exportMenu, {type: "mousemove"}, view);
const exportFile = menuPopup.querySelector("#console-menu-export-file");
ok(exportFile, "copy menu item is enabled");
var nsifile = new FileUtils.File('/tmp/test_me.log');
MockFilePicker.setFiles([nsifile]);
EventUtils.synthesizeMouseAtCenter(exportFile, {}, view);
return getFileContents('file:///tmp/test_me.log');
}
function getFileContents(file) {
return new Promise((resolve, reject) => {
const channel = NetUtil.newChannel({
uri: NetUtil.newURI(file),
loadUsingSystemPrincipal: true,
});
NetUtil.asyncFetch(channel, function(inputStream, status) {
if (Components.isSuccessCode(status)) {
info("Fetched downloaded contents.");
resolve(NetUtil.readInputStreamToString(inputStream, inputStream.available()));
} else {
reject();
}
});
});
}
it gives me the following in the terminal:
console.log: ====================================================================================================
console.log: hello test.js:4:17
myObject:
Object { a: 1 }
myArray:
Array [ "b", "c" ]
test.js:5:17
Error: "error object"
wrapper http://localhost:52054/test.js:6
logStuff http://localhost:52054/test.js:10
test.js:6:17
console.trace() myConsoleTrace test.js:7:17
wrapper http://localhost:52054/test.js:7
logStuff http://localhost:52054/test.js:10
world ! test.js:8:17
console.log: ====================================================================================================
(looks good to me :) )
So what was wrong:
- in
context-menu.js
, we were usingwebconsoleOutput.textContent
instead ofgetElementText(webconsoleOutput)
(hence losing all the line ending) - in the test, we were checking that the submenu item was in the DOM tree, and then calling
click
on it. But it wasn't visible on screen, so theclick
call wasn't doing anything. In my version, I hover theExport to
item, so display the sub item, and then I click on the item withsynthesizeMouseAtCenter
- the
read
function you had was throwing for some reason. I copied thegetFileContents
from a test in the JSONViewer, and it looks good now.
Hope this helps!
Reporter | ||
Comment 10•6 years ago
|
||
(BTW, the export to clipboard test was also failing, I guess because we didn't actually display the sub menu)
Reporter | ||
Comment 11•5 years ago
|
||
Hello Jefry,
Can I help you push this over the finish line? It'd be awesome to have this feature in the console :)
Assignee | ||
Comment 12•5 years ago
|
||
I will try to finish it up this week or the week next. As far as I can see it is just the integration test that need fixing. I will try to get it done.
Assignee | ||
Comment 13•5 years ago
|
||
hmmm.... .Ok so I think there's a confusion with the code. I think this was what happened.
You ran the code, got the error of the read() function (by this time the file was created)
Replaced it getFileContents function (it reads correctly the previously created file)
In order to correctly test this, you must delete the file in the tmp directory each time you run the tests otherwise you won't get the error.
Testing it further, I think I still have the same error. By the time firefox writes the file, I can't read it. Please check it on your end to see if it happens.
Reporter | ||
Comment 14•5 years ago
|
||
Reporter | ||
Comment 15•5 years ago
|
||
I think you're right Jefry.
So I took another look, and this seems to work https://phabricator.services.mozilla.com/D31040 .
Feel free to copy the interesting bits into your patch :)
Also, we may have a different test file for the export to file, and not reuse the one for the clipboard (or rename the test to reflect what it does).
Assignee | ||
Comment 16•5 years ago
|
||
Thanks Nicolas!, I have updated the phab diff with all the changes. I think everything works now.
Assignee | ||
Comment 17•5 years ago
|
||
Hi, Nicolas, I think everything is fine. I'm having problems pushing to the try server. Could you push the patch for me?
Reporter | ||
Comment 18•5 years ago
|
||
Jefry, I still see a windows error: https://treeherder.mozilla.org/#/jobs?repo=try&revision=929ac6169d845ff5fcf54aacc830a41a72345dc9&selectedJob=249670580
I think that's because how we split to get the lines.
I tried to fix this and pushed to TRY again https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffed474d7579aef673cf9e6e8f9a1017ae8e6245 (here's the fix: https://hg.mozilla.org/try/rev/8254ff06c2faf5eb0cf5d8f7f9db3ad583ebb3b6)
Updated•5 years ago
|
Comment 19•5 years ago
|
||
Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0883abbe6a6f Export Console content to file. .
Comment 20•5 years ago
|
||
bugherder |
Description
•