Saving a page a second time as Text changes the extension to HTML
Categories
(Firefox :: File Handling, defect, P3)
Tracking
()
People
(Reporter: jasonr9, Unassigned)
References
Details
(Whiteboard: [outreachy-2021-downloads])
Attachments
(1 file)
(deleted),
image/jpeg
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0
Steps to reproduce:
For several months now, when trying to save a webpage as text, Firefox will save text data, but add an .html extension. This occurs even if text is the option to appear when first hitting save, and I still select text as the format to save. If I hit tab and then t, however, it will work.
Comment 1•4 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #1)
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
This is not related to a web extension. This is a menu item found under, Alt>File>Save as (Ctrl+s), but am unsure of the best place so am moving it back to general.
Comment 3•4 years ago
|
||
I'm not seeing this. When I save this bug as text (selecting Text Files in the save dialog), I get:
1656573 - Issue when attempting to save a page as text.txt
Can you be more specific as to how you are saving?
(In reply to Mike Kaply [:mkaply] from comment #3)
I'm not seeing this. When I save this bug as text (selecting Text Files in the save dialog), I get:
1656573 - Issue when attempting to save a page as text.txt
Can you be more specific as to how you are saving?
If I hit ctrl-s and save as text I get an file with an html extension; the first item in the screenshot. If I hit crtl-s followed by tab, followed by r, I get an item with a text extension; the second item in the screenshot. The html file is not truly an html file; it contains the same text as the text file.
(In reply to jasonr9 from comment #6)
(In reply to Mike Kaply [:mkaply] from comment #3)
I'm not seeing this. When I save this bug as text (selecting Text Files in the save dialog), I get:
1656573 - Issue when attempting to save a page as text.txt
Can you be more specific as to how you are saving?
If I hit ctrl-s and save as text I get an file with an html extension; the first item in the screenshot. If I hit crtl-s followed by tab, followed by r, I get an item with a text extension; the second item in the screenshot. The html file is not truly an html file; it contains the same text as the text file.
corrected:
If I hit ctrl-s and save as text I get an file with an html extension; the first item in the screenshot. If I hit crtl-s followed by tab, followed by t, I get an item with a text extension; the second item in the screenshot. The html file is not truly an html file; it contains the same text as the text file.
Comment 8•4 years ago
|
||
FWIW, I can reproduce this on Windows 10 but only if the selected option is already "Text Files". If I change it to HTML and save a file, and then save it a second time, but this time change to saving text only using either the mouse or the keyboard, the file extension for the file is updated by the Windows filepicker.
On macOS, even changing the selected file type doesn't update the file extension, though I imagine that's a problem with the OS filepicker or our widget code's handling of it - but probably not directly relevant for this bug.
I think the fix for the Windows issue is likely to update the file picker code to make the suggested filename's file extension match the pre-selected filetype, which we appear to store at https://searchfox.org/mozilla-central/rev/a315a1a0f09550e23e4590a77e74f36543315da3/toolkit/content/contentAreaUtils.js#694-701 . That code is a bit weird btw, because AFAICT we can have a different number of options if saving XML files vs. HTML files, and we only store an index into those options. :-\
Anyway, we should be able to work out the "right" file extension for the default-selected picker.
Mike, can you repro with these additional details? And reporter, would you mind clarifying if, if you save a file as a dummy "HTML only" file for a change, if you then save it again and select the "Text Files" option with the mouse, the extension is correctly updated? Thanks!
Comment 9•4 years ago
|
||
Also, I think this may be a dupe of the much-duped bug 120313. :-(
Reporter | ||
Comment 10•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #8)
FWIW, I can reproduce this on Windows 10 but only if the selected option is already "Text Files". If I change it to HTML and save a file, and then save it a second time, but this time change to saving text only using either the mouse or the keyboard, the file extension for the file is updated by the Windows filepicker.
On macOS, even changing the selected file type doesn't update the file extension, though I imagine that's a problem with the OS filepicker or our widget code's handling of it - but probably not directly relevant for this bug.
I think the fix for the Windows issue is likely to update the file picker code to make the suggested filename's file extension match the pre-selected filetype, which we appear to store at https://searchfox.org/mozilla-central/rev/a315a1a0f09550e23e4590a77e74f36543315da3/toolkit/content/contentAreaUtils.js#694-701 . That code is a bit weird btw, because AFAICT we can have a different number of options if saving XML files vs. HTML files, and we only store an index into those options. :-\
Anyway, we should be able to work out the "right" file extension for the default-selected picker.
Mike, can you repro with these additional details? And reporter, would you mind clarifying if, if you save a file as a dummy "HTML only" file for a change, if you then save it again and select the "Text Files" option with the mouse, the extension is correctly updated? Thanks!
Yes, if I see text when I start and then select text it will not work, but if I see html when I start and then select text it will work.
Comment 11•4 years ago
|
||
Yes, I can recreate in this case. It only happens if the save type is already text. So you have to save as Text once (extensions is correct), then the second time it is incorrect.
The code in question is here:
https://searchfox.org/mozilla-central/source/toolkit/content/contentAreaUtils.js#682
This should just be a matter of checking aFpP.saveMode before setting the default extension.
Comment 12•4 years ago
|
||
This should just be a matter of checking aFpP.saveMode before setting the default extension.
We don't have the previous mode, so we can't check that. This is probably going to be an issue in the file picker code itself.
Comment 13•4 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #12)
This should just be a matter of checking aFpP.saveMode before setting the default extension.
We don't have the previous mode, so we can't check that. This is probably going to be an issue in the file picker code itself.
I'm not sure I follow. AFAICT we could move the block that does:
// The index of the selected filter is only preserved and restored if there's
// more than one filter in addition to "All Files".
if (aFpP.saveMode != SAVEMODE_FILEONLY) {
// eslint-disable-next-line mozilla/use-default-preference-values
try {
fp.filterIndex = prefBranch.getIntPref("save_converter_index");
} catch (e) {}
}
up, and based on the filterIndex and the list of appended filters, we can determine what save mode is selected, and we can then set defaultString differently based on the selected extension, instead of the default file extension? What am I missing?
Comment 14•4 years ago
|
||
aFpP.saveMode doesn't appear to be the previous save mode. I checked. It's still 3 when you've saved TXT before.
So I'm not even convinced that code you've quoted works.
Comment 15•4 years ago
|
||
Yeah, I just checked. It's always 3 regardless of what you did with your previous save.
Comment 16•4 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #15)
Yeah, I just checked. It's always 3 regardless of what you did with your previous save.
saveMode
will be 3 but filterIndex
won't be, right? Otherwise, I don't see why the file picker would even select a different default filetype.
Comment 17•4 years ago
|
||
You're right, filterIndex is the key here, and as you pointed out, we're not using it to affect the extension. So that would be the thing to do.
Also, side note, this code is used to save images on web pages as well, so we'll want to make sure that doesn't get broken. While investigating this, I discovered we don't save webp images properly and I opened a bug for that.
Updated•4 years ago
|
Comment 18•4 years ago
|
||
I keep not finding time for this, so unassigning. However, we can probably mentor someone to do this.
Comment 19•4 years ago
|
||
I'm an Outreachy applicant and I want to work on "Improve the Firefox Downloads Panel" project. I would like to work on this issue. I'm new to version controls, so if anyone would be able to guide me on how I can work on bugs, it would be very helpful. Thank you.
Comment 20•4 years ago
|
||
(In reply to Sadaf Fateema from comment #19)
I'm an Outreachy applicant and I want to work on "Improve the Firefox Downloads Panel" project. I would like to work on this issue. I'm new to version controls, so if anyone would be able to guide me on how I can work on bugs, it would be very helpful. Thank you.
Hi Sadaf, do you have your local Firefox build set up? That would be the first thing to do.
Comment 21•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #20)
(In reply to Sadaf Fateema from comment #19)
I'm an Outreachy applicant and I want to work on "Improve the Firefox Downloads Panel" project. I would like to work on this issue. I'm new to version controls, so if anyone would be able to guide me on how I can work on bugs, it would be very helpful. Thank you.
Hi Sadaf, do you have your local Firefox build set up? That would be the first thing to do.
Yes, I have Firefox setup on my system.
Comment 22•4 years ago
|
||
(In reply to Sadaf Fateema from comment #21)
Yes, I have Firefox setup on my system.
OK, then the first step would be reproducing the issue as it's been reported (see also comment #8) on your own local Firefox build. Then you can look at comment #13, and try fixing the issue by moving the code from https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/toolkit/content/contentAreaUtils.js#701-708 further up in the method, and changing the extension we use to create fp.defaultString
based on the filterIndex
. Specifically, you would probably want to pass the index into the appendFiltersForContentType
helper at https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/toolkit/content/contentAreaUtils.js#841 and have the helper return the selected extension, and then use that at the callsite.
You can debug the execution of the code using the debugger in the browser toolbox ( https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox ) and/or console.log statements in the code which will show up in the browser console ( https://developer.mozilla.org/en-US/docs/Tools/Browser_Console ).
Does that sound doable?
Comment 23•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #22)
(In reply to Sadaf Fateema from comment #21)
Yes, I have Firefox setup on my system.
OK, then the first step would be reproducing the issue as it's been reported (see also comment #8) on your own local Firefox build. Then you can look at comment #13, and try fixing the issue by moving the code from https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/toolkit/content/contentAreaUtils.js#701-708 further up in the method, and changing the extension we use to create
fp.defaultString
based on thefilterIndex
. Specifically, you would probably want to pass the index into theappendFiltersForContentType
helper at https://searchfox.org/mozilla-central/rev/9043e515e9608cc55b252a40cb2dfb6f767bcffd/toolkit/content/contentAreaUtils.js#841 and have the helper return the selected extension, and then use that at the callsite.You can debug the execution of the code using the debugger in the browser toolbox ( https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox ) and/or console.log statements in the code which will show up in the browser console ( https://developer.mozilla.org/en-US/docs/Tools/Browser_Console ).
Does that sound doable?
I have reproduced the issue too. I'm a little lost on how to edit the code on my system.
I'll try my best to get it done.
Comment 24•4 years ago
|
||
hey I would like to take up on this issue ,if it is unassigned.
Thanks
Comment 25•4 years ago
|
||
Hey! I am an outreachy applicant and am looking for my first contribution. Can I work on this bug?
Comment 26•4 years ago
|
||
(In reply to Sadaf Fateema from comment #19)
I'm an Outreachy applicant and I want to work on "Improve the Firefox Downloads Panel" project. I would like to work on this issue. I'm new to version controls, so if anyone would be able to guide me on how I can work on bugs, it would be very helpful. Thank you.
Sadaf, can you please confirm if you are working on this bug or not?
Thanks.
Updated•4 years ago
|
Comment 27•4 years ago
|
||
Hi Gijs! I'm sorry I wasn't able to work on this sooner due to covid-related complications in my country. But I've began working on it. I've understood what the issue is and what I need to do. It's just I'm not able to understand what does the "filterIndex" do? How does it relate to the extension? I'm also little confused regarding using the debugger to see the execution of the code. Do I have to use it for the "Save As" dialog box? How do I find the specific file to use it upon? How to use the debugger for this?
Updated•4 years ago
|
Comment 28•4 years ago
|
||
Hi Anshul!
(In reply to Anshul Sahai from comment #27)
what does the "filterIndex" do? How does it relate to the extension?
See: https://searchfox.org/mozilla-central/source/widget/nsIFilePicker.idl#126
I believe the filterIndex
here is referring to the index of the selected item of the filters list. This "filters list" is the dropdown list you see beside the "Save as Type" on Windows when the file picker dialog is open. You can get this file picker to show when selecting the "Save Page As..." option after right-clicking a web page.
Now, as for how it's related to the extension, it looks like the selected filter affects what type of file you're saving the resource as. So if you're trying to save the webpage as a .txt
file, you'd have to select the "Text Files" item from the filters list (which also has an filterIndex
of 2).
Do I have to use it for the "Save As" dialog box?
Yes, the filterIndex
should be used by the dialog box opened by "Save As..." options.
How do I find the specific file to use it upon? How to use the debugger for this?
You can open the debugger for the browser by opening the Browser Toolbox and going to the Debugger tab.
From there, I would trace how the filters list is built by setting some breakpoints (or console logs, if you prefer) in the appendFiltersForContentType function.
Another good place to start debugging the code is also observing how the selected filter is set, which I think is around here: https://searchfox.org/mozilla-central/rev/71515d047cbdd02687a1b9b7265f2ffb51300bf1/toolkit/content/contentAreaUtils.js#719-721.
I think once you've got a little familiar with how some of these things are working by debugging some of the above, it would be a good idea to revisit Gijs' suggestion in comment 22 and see if you can fix the bug from there.
Comment 32•3 years ago
|
||
Hey Gijs!
I didn't get the chance to work on it. I hope someone else can take it up for now otherwise I'll try working on it soon. :)
Comment 33•3 years ago
|
||
(In reply to Anshul Sahai from comment #32)
Hey Gijs!
I didn't get the chance to work on it. I hope someone else can take it up for now otherwise I'll try working on it soon. :)
OK, I'll unassign you for now. :-)
Description
•