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

Avalonia: How do properly handle file downloads? #379

Open
dhhunter opened this issue Jan 30, 2025 · 10 comments · May be fixed by #380
Open

Avalonia: How do properly handle file downloads? #379

dhhunter opened this issue Jan 30, 2025 · 10 comments · May be fixed by #380

Comments

@dhhunter
Copy link

Greetings, as the question states how do you properly handle file downloads?

Thanks and God Bless,
- Dean

@dhhunter
Copy link
Author

dhhunter commented Jan 30, 2025

It seems that adding an event handler to any of the Download events (DownloadCancelled, DownloadCompleted, DownloadProgressChanged) throws a NullReferenceException in InternalDownloadHandler.OnDownloadUpdated, the problem appears to be with the CefDownloadItem! The CefDownloadItem is being Disposed() before OnDownloadUpdated can use it. To my pleasant surprise downloads work perfectly fine as fire and forget operations (not using DownloadCancelled, DownloadCompleted, DownloadProgressChanged); however, for my use case I really need to be able to track the download so I did a little debugging.

I believe that a simple change to WebView.InternalDownloadHandler like below will solve the problem:

protected override void OnDownloadUpdated(CefBrowser browser, CefDownloadItem downloadItem, CefDownloadItemCallback callback) 
{
    var fullPath = downloadItem.FullPath;
    var receivedBytes = downloadItem.ReceivedBytes;
    var totalBytes = downloadItem.TotalBytes;

    if (downloadItem.IsComplete) {
        if (OwnerWebView.DownloadCompleted != null) {
            OwnerWebView.AsyncExecuteInUI(() => OwnerWebView.DownloadCompleted?.Invoke(fullPath));
        }
    } else if (downloadItem.IsCanceled) {
        if (OwnerWebView.DownloadCancelled != null) {
            OwnerWebView.AsyncExecuteInUI(() => OwnerWebView.DownloadCancelled?.Invoke(fullPath));
        }
    } else {
        if (OwnerWebView.DownloadProgressChanged != null) {
            OwnerWebView.AsyncExecuteInUI(() => OwnerWebView.DownloadProgressChanged?.Invoke(fullPath, receivedBytes, totalBytes));
        }
    }
}

Thanks and God Bless,
- Dean

@joaompneves
Copy link
Collaborator

Can you submit a PR?

@dhhunter
Copy link
Author

dhhunter commented Feb 3, 2025

I have never submitted a PR before; however, I doubt it is that difficult! I am generally kind of hesitant to work on other peoples code; however, it would probably be a good exercise for me. I will work on that today!

Thanks!

@dhhunter
Copy link
Author

dhhunter commented Feb 3, 2025

It occurred to me that the way the API is designed tracking multiple downloads is potentially a little wonky since the downloadItem.Id is not sent as a parameter to the public download events. I do not want to break the public API so I will leave that decision for the project owners!

For now, I suppose a suitable work around would probably be to delay tracking (on the UI side anyway) the download until the resourcePath != "", especially in more advanced scenarios. Anyway, my further testing is delaying my PR!

@joaompneves
Copy link
Collaborator

joaompneves commented Feb 4, 2025

You can add events similiar to the existing ones, but with extra id param, so that we don't have breaking changes. I would probably create a class/record to put that data so that in the future this problem doesn't happen

@dhhunter
Copy link
Author

dhhunter commented Feb 8, 2025

@joaompneves Thanks! My use case is not impossible with the current events; however, having the id parameter makes it trivial while you really have to dig in and figure out what is going on with the old events! Now if only I could decide what to call the new events, I really don't like the names I am using right now! Lol

I have also been leaning towards packaging the params into an object, so that sounds like the way to go!

@dhhunter
Copy link
Author

dhhunter commented Feb 10, 2025

@joaompneves I tried creating a new branch in Visual Studio and committing my updates so far into it; however, it seems to only allow me to do so locally as I do not have permissions to do anything with the remote github. How do I make my code available for review as I think I am almost done?

I am just not sure if I want to leave the object as WebView.DownloadItem or just make it simply DownloadItem.

I also have one last quandary that I have yet to figure out, I need my application to respond to web pages that try to open links in new browser windows and instead open them in a new tab inside of my application... Any ideas? I figured out that this is what the OnPopupOpening event is for! :)

@dhhunter
Copy link
Author

@joaompneves Alright, I am ready to move forward with my first pull request! The article that I was reading says that I am supposed to create a new branch in the repo (which I did locally) that I should be able to use like any other repo; unfortunately, I do not have access to actually save my changes to the remote branch so that anyone else can review them! Once I get my changes saved into the remote branch then the article said that I create the actual PR to actually get you all to review and merge my changes into the Master branch? I am excited and yet also very nervous!

@joaompneves
Copy link
Collaborator

dhhunter added a commit to dhhunter/WebView that referenced this issue Feb 12, 2025
Fix disposed error in Download Events & added DownloadItem Events (Fix OutSystems#379)!
Added GetText() (allows integration with AI assistants) & GetSource()!
F12 now actually TogglesDeveloperTools!
Fixed sample menu and polished UI!
OCD corrected typos in some internal variables (sorry)!
@dhhunter
Copy link
Author

dhhunter commented Feb 12, 2025

@joaompneves Oh, okay! Thank you! In the process I majorly messed up my forks repo, all I wanted to do was go back and edit my original comment; however, I think that I finally filed the PR. I am all sorts of panicked over it, hopefully my repo mishaps will not affect your repo when merged. Surely, an easier way exists to edit the comment on a commit... Or would it be better if I deleted my fork and started over with a clean fork? It really ultra bothers me that my repo is goofed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants