Skip to content
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

BeforeDownloadStarting event #2890

Open
Optimierungswerfer opened this issue Oct 18, 2022 · 8 comments
Open

BeforeDownloadStarting event #2890

Optimierungswerfer opened this issue Oct 18, 2022 · 8 comments
Labels
feature request feature request priority-low We have considered this issue and decided that we will not be able to address it in the near future. tracked We are tracking this work internally.

Comments

@Optimierungswerfer
Copy link

Optimierungswerfer commented Oct 18, 2022

Is your feature request related to a problem? Please describe.
Say I want to intercept some type of file when downloading using the DownloadStarting event and change the local download path for it using CoreWebView2DownloadStartingEventArgs::ResultPath(newPath) and I want to keep the original file name. First, I would parse the filename from the original ResultPath, then combine that with the desired directory, and then set it as the new ResultPath. So far so good.

However, the download is already beginning concurrently in the background using default download settings by the time the DownloadStarting event gets fired. The issue now is that when in the default download location (e.g. the user's download folder) there is already a file with the same name, so WebView2 adds an index to the filename like "MyDownload (1).txt". Even though I want to save the file in a totally different location where there's no conflicting file, I get the index in the filename because WebView2 already evaluated the default download location and started the download. Then, in the DownloadStarting event I am presented only with the already modified filename. There is no way to tell if such a rename has occurred, because there is no indicator for that and the original file also may have actually been called "MyDownload (1).txt".

Another issue with the DownloadStarting event is the race condition regarding the event handlers that can be registered there for the DownloadOperation, which is described in issue #2180.

Describe the solution you'd like and alternatives you've considered
Either a completely new event BeforeDownloadStarting which has to finish before WebView2 starts downloading anything in the background which allows the developer to configure the download settings such as the local target directory and event handlers for the DownloadOperation before the download starts or - as a more mundane solution solving this particular case - two new attributes inside CoreWebView2DownloadStartingEventArgs like bool IsResultFilepathModified and hstring OriginalResultFilePath which would indicate if the ResultFilePath was modified and what its original value has been.
I definitely like the first solution better, since it also provides the possibility of registering event handlers for the upcoming DownloadOperation which will definitely get executed during the download and not be subject to a race condition.

AB#41838437

@Optimierungswerfer Optimierungswerfer added the feature request feature request label Oct 18, 2022
@nishitha-burman nishitha-burman added the tracked We are tracking this work internally. label Oct 18, 2022
@nishitha-burman
Copy link
Collaborator

Thank you for the feedback! We've added this to our backlog.

@darbid
Copy link

darbid commented Oct 19, 2022

@Optimierungswerfer aka Opto Thrower :-)

you haven't by chance been using CefSharp? It's API gave a much earlier notification of downloads. It seems Chrome will start a download before knowing the file name and CefSharp exposed that too. Confirming a file name came while the download was in progress. This also meant that you could actually show download progress.

With WebView2 downloadstarting is kinda a meme really, it feels like the download has pretty much completed on the client pc by the time this fires.

WebView2's implementation is certainly more of a "retail" event, nice and easy, all the info is there at start and away you go. However, at the risk of making it more complicated I am with you that I would like to know a lot earlier about the download.

Race condition is def an issue, I think.

@nishitha-burman
Copy link
Collaborator

Hello,

Can you please provide more context on the scenario you are trying to address? Are you saying that you would like to change the download path after the DownloadStarting event has fired?

Thanks!

@Optimierungswerfer
Copy link
Author

Precisely. Depending on the file type, we want to change the download path when a download occurs, before it starts. Because otherwise, the file name gets "polluted" with an index should there already be a file of the same name in the default download location, where we don't even want to put the download. In the path where we actually want to download to, we can be sure that there is no name conflict.

Our current workaround is a bit cumbersome: We set the default download location for all downloads CoreWebView2Profile::DefaultDownloadFolderPath to the path where we want to put files of that specific file type, and for all other file types we use the CoreWebView2DownloadStartingEventArgs::ResultPath property to change their path back to the default download location, which in turn can result in a name conflict that we now have to manually resolve beforehand.

@Optimierungswerfer
Copy link
Author

So I stumbled upon the HTTP-Response-Header Content-Disposition which optionally includes a "filename" directive that the CoreWebView2DownloadOperation class exposes and is left unchanged even when the ResultPath property had been modified with an index.

This definitely helps my situation, however it is not a complete solution, because as mentioned it is an optional directive. Also, the race condition for e.g. DownloadStateChanged event handler registration remains an issue.

@nishitha-burman
Copy link
Collaborator

To confirm, the main reason for this is to avoid having files with the same name? If there are name conflicts what do you do currently? Can you describe step-by-step what you are hoping to achieve with the BeforeDownloadStarting event? Can you explain why it is not sufficient to set a download location different from the default?

Thank!

@Optimierungswerfer
Copy link
Author

What we are doing is integrating our web product via WebView with our Windows editor for documents managed through the web product. We want to seamlessly download a document file in the background to a temporary location, load it into our editor's memory, and delete the file. This process requires two things: 1. A properly working DownloadStateChanged event (so we can open the file once the download completes) and 2. the filename must not be altered during the download process (the editor must receive the exact filename that was sent by our web product).

The issue with 1 is that the event handlers for DownloadStateChanged do not fire when the download has already finished before we even had the chance to register them in the DownloadStarting event.

The issue with 2 is that WebView starts downloading any download to the user's default download folder before we even get to the DownloadStarting event, where we have the possibility to alter the ResultPath to point to our temporary location. However, at this point, the file name might have already been altered to contain an index due to WebView's automatic conflict resolution when first trying to create the file in the user's default download folder. But we need the unaltered filename in our temporary location and our editor. We work around this particular issue now by parsing the Content-Disposition header, which still contains the original filename instead of retrieving it from the ResultPath, that WebView already altered. I guess this header is where WebView gets the filename for the download in the first place?

Both of these issues could be fixed by providing an event where we can cleanly configure the download that's about to start before it actually starts; Register our StateChanged event handlers, so they are definitely being called for all state changes of the download. Set the ResultPath for that particular download (instead of e.g. changing it globally via the CoreWebView2Profile.DefaultDownloadFolderPath property, which has other undesired effects) before it starts so that nothing is tried to be written to the default download folder path in the first place, avoiding any filename conflicts and unnecessary resolutions altogether.

@github-actions github-actions bot added the priority-low We have considered this issue and decided that we will not be able to address it in the near future. label Mar 26, 2024
@Optimierungswerfer
Copy link
Author

I figured out why the DownloadStateChanged event did not fire: #2180 (comment)

It would be nice if this unintuitive behavior could be changed or at least documented. But at least that resolves the first of the two issues mentioned here.

However, the second issue ("polluted" download file name) remains. As mentioned in my initial post here, this could also be solved by just adding a read-only field inside CoreWebView2DownloadStartingEventArgs like bool IsResultFilepathModified and hstring OriginalResultFilePath which would indicate if the ResultFilePath was modified and what its original value has been. Seeing as this is the only remaining issue, this simpler solution would suffice to eliminate the need for a BeforeDownloadStarting event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request feature request priority-low We have considered this issue and decided that we will not be able to address it in the near future. tracked We are tracking this work internally.
Projects
None yet
Development

No branches or pull requests

3 participants