Skip to content

refactor: remove or replace deprecated methods in engine#1963

Open
jcesarmobile wants to merge 2 commits into
masterfrom
remove-deprecated
Open

refactor: remove or replace deprecated methods in engine#1963
jcesarmobile wants to merge 2 commits into
masterfrom
remove-deprecated

Conversation

@jcesarmobile

Copy link
Copy Markdown
Member

onExceededDatabaseQuota was deprecated on Android API 19 and has no effect https://developer.android.com/reference/android/webkit/WebChromeClient#onExceededDatabaseQuota(java.lang.String,%20java.lang.String,%20long,%20long,%20long,%20android.webkit.WebStorage.QuotaUpdater)

shouldOverrideUrlLoading was deprecated on Android API 24, replaced it with shouldOverrideUrlLoading ([WebView](https://developer.android.com/reference/android/webkit/WebView) view, [WebResourceRequest](https://developer.android.com/reference/android/webkit/WebResourceRequest) request)
https://developer.android.com/reference/android/webkit/WebViewClient#shouldOverrideUrlLoading(android.webkit.WebView,%20java.lang.String)

onReceivedError was deprecated on Android API 24, replaced it with onReceivedError ([WebView](https://developer.android.com/reference/android/webkit/WebView) view, [WebResourceRequest](https://developer.android.com/reference/android/webkit/WebResourceRequest) request, [WebResourceError](https://developer.android.com/reference/android/webkit/WebResourceError) error)
https://developer.android.com/reference/android/webkit/WebViewClient#onReceivedError(android.webkit.WebView,%20int,%20java.lang.String,%20java.lang.String)

shouldInterceptRequest was deprecated on Android API 21, we already have the newer method implemented, so just deleted the old one.
But note that the removed one had some checks that we no longer do such as shouldAllowRequest.

setGeolocationDatabasePath was deprecated on Android API 24, while the comment says "// @todo remove when Cordova drop API level 24 support", if it's deprecated in 24 we should not use it in 24.

Not sure if it could be considered breaking, pretty sure shouldOverrideUrlLoading and onReceivedError have no effect at the moment and introducing the non deprecated ones might change behaviors, but they are old behaviors that are not working at the moment.

@codecov-commenter

codecov-commenter commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.43%. Comparing base (dd3fd35) to head (5bf0d55).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1963   +/-   ##
=======================================
  Coverage   61.43%   61.43%           
=======================================
  Files          24       24           
  Lines        4922     4922           
=======================================
  Hits         3024     3024           
  Misses       1898     1898           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@erisu

erisu commented Jun 29, 2026

Copy link
Copy Markdown
Member

Regards to comment:

Not sure if it could be considered breaking, pretty sure shouldOverrideUrlLoading and onReceivedError have no effect at the moment and introducing the non deprecated ones might change behaviors, but they are old behaviors that are not working at the moment.

I am also not sure if this change should be considered breaking or a bug fix.

The change could be seen as a fix for a regression, since functionality was effectively lost and this restores what would generally be considered the expected behavior.

But also give that it has been 3 years since the change, maybe it could be breaking?

There would be a behavior change.

For example, if the application tries to load JavaScript files that don't exist, the following dialog will be displayed with the changes in this PR:

onReceivedError

The application will also close when you click OK. I vaguely remember this being the behavior previously.

In the current release, for the past 3 years where this functionality was lost, the application silently ignores the failure but logs the following message to the console:

Failed to load resource: net::ERR_CONNECTION_REFUSED

Should we split the PR? One PR being the refactoring/cleanup work, and another could reintroduce shouldOverrideUrlLoading and onReceivedError.

@dpogue

dpogue commented Jun 29, 2026

Copy link
Copy Markdown
Member

Arguably this is a bug fix, but if someone wanted to hold it until the next major I wouldn't be opposed to that. At the very least, it should be a minor not a patch.

My other thought was to temporarily enable the new behaviour only in debug builds, but that gets messy and risks other bugs that are hard to track down and troubleshoot.

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 this pull request may close these issues.

4 participants