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

Added download event #743

Closed
wants to merge 5 commits into from
Closed

Conversation

shaikh-amaan-fm
Copy link
Contributor

@shaikh-amaan-fm shaikh-amaan-fm commented Jun 29, 2020

Platforms affected

Android

Motivation and Context

This change enables, generation of a download event.

closes #311

Description

Whenever the InAppBrowser receives or locates to a url which leads in downloading a file, the callback assgined to the "download" event is called. The parameter passed to this callback is an object with the the follwing properties

  • type it contains the String value "download" always
  • url The url that leaded to the downloading of file. Basically, the download link of file
  • userAgent User Agent of the webview
  • contentDisposition If the url contains "content-disposition" header, then this property holds the value of that field else this field is empty
  • contentLength If the link of the file allows to obtain file size then this property holds the file size else it contains int value 0
  • mimetype The MIME type of the file

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution.

I left a few comments but due to the nature of this PR, I would also invite you to join our dev mailing list for discussion on Cordova development. I'm personally hesitate to merge any new features that only supports a single platform, unless if there is a good reason for it.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/android/InAppBrowser.java Outdated Show resolved Hide resolved
src/android/InAppBrowser.java Outdated Show resolved Hide resolved
src/android/InAppBrowser.java Outdated Show resolved Hide resolved
www/inappbrowser.js Outdated Show resolved Hide resolved
@veelci
Copy link

veelci commented Sep 13, 2023

@breautek Is there any chance of this getting merged in? I'd like to avoid forking this repo to enable file downloads on android.

src/android/InAppBrowser.java Show resolved Hide resolved
@breautek
Copy link
Contributor

@breautek Is there any chance of this getting merged in? I'd like to avoid forking this repo to enable file downloads on android.

I can't really guarantee it, but overall I think this PR is good. I invited Nik for a second pair of eyes since he frequently uses/maintains this plugin... and the inappbrowser isn't something I actually use in my own projects.

Assuming that he's okay with it, I'll then allocate some time to rebase the PR to get it merged. Unless if @shaikh-amaan-fm is still around and does that rebase first.

My concern back in 2020 about it being android only still applies, but it isn't really a deal breaker, imo.

@veelci
Copy link

veelci commented Sep 13, 2023

Thank you for the quick response. This should resolve issue #311 .

Copy link
Member

@NiklasMerz NiklasMerz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit I haven't worked on an app with the IAB plugin in a long time but the code looks good to me. I think we can merge it if someone gets to use it.

@breautek
Copy link
Contributor

Thanks Nik, I should have some time tonight after my work day to spend some time rebasing, and it will give me a chance to actually test the code as well.

So the plan is to merge sometime later tonight.

@breautek breautek mentioned this pull request Sep 14, 2023
5 tasks
@breautek breautek mentioned this pull request Sep 15, 2023
@breautek breautek added this to the 5.1.0 milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download file not working
4 participants