New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: modify file extension generation on Windows #34723
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there's still a bit of strange behavior here when showing extension and switching between potential save file types 🤔
If I open the html file from this gist in Edge on Windows, right click, and choose "Save Link As" - we get the following popup:
The title of the item doesn't change if I then switch the file type to All Files
:
I would expect that if I ran the fiddle and clicked the "download" link, I would see a similar popup. At first, the behavior matches:
However, if I switch the file save type to All Files
, the title logic gets a little confused:
and then if I switch back to PDF File
, disappears altogether:
I'd ideally like to see these match for a more intuitive user experience, and the i think we can do a final review for this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(oops see above, meant to req changes!)
Co-authored-by: Charles Kerr <charles@charleskerr.com>
No Release Notes |
* fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com>
* fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com>
I have automatically backported this PR to "18-x-y", please check out #35171 |
I have automatically backported this PR to "19-x-y", please check out #35172 |
I have automatically backported this PR to "20-x-y", please check out #35173 |
fix: modify file extension generation on Windows (#34723) * fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: Michaela Laurencin <35157522+mlaurencin@users.noreply.github.com> Co-authored-by: Charles Kerr <charles@charleskerr.com>
fix: modify file extension generation on Windows (#34723) * fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: Michaela Laurencin <35157522+mlaurencin@users.noreply.github.com> Co-authored-by: Charles Kerr <charles@charleskerr.com>
* fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com>
* fix: modify file extension generation on Windows * modify includes * include vector in header * add win build flags * remove hardcoded strings * Update shell/browser/electron_download_manager_delegate.h Co-authored-by: Charles Kerr <charles@charleskerr.com> * fix string manipulation and function definitions * Update electron_download_manager_delegate.h * convert to std::string and modify for electron * Update shell/browser/electron_download_manager_delegate.cc Co-authored-by: Charles Kerr <charles@charleskerr.com> * remove vector include and update conversion * add vectr include for lint Co-authored-by: Charles Kerr <charles@charleskerr.com>
Description of Change
Closes #34450
Previously when saving a file on Windows, the file type would not be defined based on the extension of the file. (The linked issue also mentions that the file name should include the extension, but I could not replicate this on non-Electron applications). The file dialog will now properly display the file type and the saved file will include the extension.
Checklist
npm test
passesRelease Notes
Notes: None