Open Bug 1656573 Opened 4 years ago Updated 2 years ago

Saving a page a second time as Text changes the extension to HTML

Categories

(Firefox :: File Handling, defect, P3)

79 Branch
Desktop
All
defect

Tracking

()

People

(Reporter: jasonr9, Unassigned)

References

Details

(Whiteboard: [outreachy-2021-downloads])

Attachments

(1 file)

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.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Product: Firefox → WebExtensions

(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.

Component: Untriaged → General
Product: WebExtensions → Firefox

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?

Needinfo for comment 3 :-)

Flags: needinfo?(jasonr9)
Attached image without and with tab then t (deleted) —
Flags: needinfo?(jasonr9)

(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.

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!

Flags: needinfo?(mozilla)
Flags: needinfo?(jasonr9)

Also, I think this may be a dupe of the much-duped bug 120313. :-(

(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.

Flags: needinfo?(jasonr9)

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.

Status: UNCONFIRMED → NEW
Component: General → File Handling
Ever confirmed: true
Flags: needinfo?(mozilla)
Summary: Issue when attempting to save a page as text → Saving a page a second time as Text changes the extension to HTML

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.

(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?

Flags: needinfo?(mozilla)

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.

Flags: needinfo?(mozilla)

Yeah, I just checked. It's always 3 regardless of what you did with your previous save.

(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.

Flags: needinfo?(mozilla)

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.

Flags: needinfo?(mozilla)
Assignee: nobody → gijskruitbosch+bugs
Severity: -- → S3
Status: NEW → ASSIGNED
Priority: -- → P1

I keep not finding time for this, so unassigning. However, we can probably mentor someone to do this.

Assignee: gijskruitbosch+bugs → nobody
Status: ASSIGNED → NEW
OS: Unspecified → All
Priority: P1 → P3
Hardware: Unspecified → Desktop
Whiteboard: [outreachy-2021-downloads]

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.

(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.

Flags: needinfo?(sadaffateema1)

(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.

Flags: needinfo?(sadaffateema1)

(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?

Flags: needinfo?(sadaffateema1)

(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 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?

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.

Flags: needinfo?(sadaffateema1)

hey I would like to take up on this issue ,if it is unassigned.
Thanks

Hey! I am an outreachy applicant and am looking for my first contribution. Can I work on this bug?

(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.

Assignee: nobody → asahai100

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?

Flags: needinfo?(gijskruitbosch+bugs)

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.

Thanks Micah!

Flags: needinfo?(gijskruitbosch+bugs)

Anshul, are you still working on this?

Flags: needinfo?(asahai100)

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. :)

Flags: needinfo?(asahai100)

(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. :-)

Assignee: asahai100 → nobody
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: