You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by "jcesarmobile (via GitHub)" <gi...@apache.org> on 2026/06/07 14:08:49 UTC
[PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
jcesarmobile opened a new pull request, #1951:
URL: https://github.com/apache/cordova-android/pull/1951
Android Studio was presenting several warnings about:
- variables that could be final
- variables that could be local
- redundant local variables
- use lambdas instead of anonymous functions
- missing descriptions
- unused variables
and a few others
I addressed them all
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "jcesarmobile (via GitHub)" <gi...@apache.org>.
jcesarmobile commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3420170816
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
Review Comment:
I would prefer to remove it, in a separate PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3373007831
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -173,6 +160,7 @@ public void onExceededDatabaseQuota(String url, String databaseIdentifier, long
long totalUsedQuota, WebStorage.QuotaUpdater quotaUpdater)
{
LOG.d(LOG_TAG, "onExceededDatabaseQuota estimatedSize: %d currentQuota: %d totalUsedQuota: %d", estimatedSize, currentQuota, totalUsedQuota);
+ long MAX_QUOTA = 100 * 1024 * 1024;
quotaUpdater.updateQuota(MAX_QUOTA);
Review Comment:
Honestly MAX_QUOTA does not explain using 100 MB here. Why is 100 MB the max quota? What will happen if this function is called again, after 100 MB was set? I think a comment would better suit here an explain the reason.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu merged PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "breautek (via GitHub)" <gi...@apache.org>.
breautek commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3370716536
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -173,6 +160,7 @@ public void onExceededDatabaseQuota(String url, String databaseIdentifier, long
long totalUsedQuota, WebStorage.QuotaUpdater quotaUpdater)
{
LOG.d(LOG_TAG, "onExceededDatabaseQuota estimatedSize: %d currentQuota: %d totalUsedQuota: %d", estimatedSize, currentQuota, totalUsedQuota);
+ long MAX_QUOTA = 100 * 1024 * 1024;
quotaUpdater.updateQuota(MAX_QUOTA);
Review Comment:
I disagree -- having a named variable can help describe intent since it avoids [magic numbers](https://en.wikipedia.org/wiki/Magic_number_(programming)).
So having `MAX_QUOTA` here is more preferable in my opinion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#issuecomment-4642915752
## [Codecov](https://app.codecov.io/gh/apache/cordova-android/pull/1951?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 61.43%. Comparing base ([`f4ecf99`](https://app.codecov.io/gh/apache/cordova-android/commit/f4ecf9974d5d5b944cba269b271cfa7a87ce1c1b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f38104c`](https://app.codecov.io/gh/apache/cordova-android/commit/f38104cf5049746bbdede575995161fd55bec818?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1951 +/- ##
=======================================
Coverage 61.43% 61.43%
=======================================
Files 24 24
Lines 4922 4922
=======================================
Hits 3024 3024
Misses 1898 1898
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/cordova-android/pull/1951?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3420350743
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3369499530
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -73,21 +73,16 @@ private interface PermissionListener {
private PermissionListener permissionListener;
private static final int FILECHOOSER_RESULTCODE = 5173;
private static final String LOG_TAG = "SystemWebChromeClient";
- private long MAX_QUOTA = 100 * 1024 * 1024;
protected final SystemWebViewEngine parentEngine;
// the video progress view
private View mVideoProgressView;
- private CordovaDialogsHelper dialogsHelper;
- private Context appContext;
-
- private WebChromeClient.CustomViewCallback mCustomViewCallback;
- private View mCustomView;
+ private final CordovaDialogsHelper dialogsHelper;
public SystemWebChromeClient(SystemWebViewEngine parentEngine) {
this.parentEngine = parentEngine;
- appContext = parentEngine.webView.getContext();
+ Context appContext = parentEngine.webView.getContext();
dialogsHelper = new CordovaDialogsHelper(appContext);
Review Comment:
I think the temporary variable `appContect` is not needed here and `new CordovaDialogsHelper` could be directly called with `parentEngine.webView.getContext()`:
```java
dialogsHelper = new CordovaDialogsHelper(parentEngine.webView.getContext());
```
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -173,6 +160,7 @@ public void onExceededDatabaseQuota(String url, String databaseIdentifier, long
long totalUsedQuota, WebStorage.QuotaUpdater quotaUpdater)
{
LOG.d(LOG_TAG, "onExceededDatabaseQuota estimatedSize: %d currentQuota: %d totalUsedQuota: %d", estimatedSize, currentQuota, totalUsedQuota);
+ long MAX_QUOTA = 100 * 1024 * 1024;
quotaUpdater.updateQuota(MAX_QUOTA);
Review Comment:
I think the variable assignment `MAX_QUOTA` is redundant here. I would suggest this code:
```java
quotaUpdater.updateQuota(100 * 1024 * 1024);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "breautek (via GitHub)" <gi...@apache.org>.
breautek commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3390618664
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -173,6 +160,7 @@ public void onExceededDatabaseQuota(String url, String databaseIdentifier, long
long totalUsedQuota, WebStorage.QuotaUpdater quotaUpdater)
{
LOG.d(LOG_TAG, "onExceededDatabaseQuota estimatedSize: %d currentQuota: %d totalUsedQuota: %d", estimatedSize, currentQuota, totalUsedQuota);
+ long MAX_QUOTA = 100 * 1024 * 1024;
quotaUpdater.updateQuota(MAX_QUOTA);
Review Comment:
> Why is 100 MB the max quota?
That part is obviously subjective.
> What will happen if this function is called again, after 100 MB was set?
This on the on exceed hook, which means the user has already exceeded the previously set quota, whatever what that might have been. I think the default is ~5 megabytes, but that's a webview implementation detail that's not documented.
If the user exceeds the quota and the quota is not expandable then they'll hit the standard [QuotaExceededError](https://developer.mozilla.org/en-US/docs/Web/API/QuotaExceededError).
Also worth noting that this whole system is deprecated and apparently no longer used since API 19 (according to https://developer.android.com/reference/android/webkit/WebChromeClient#onExceededDatabaseQuota(java.lang.String,%20java.lang.String,%20long,%20long,%20long,%20android.webkit.WebStorage.QuotaUpdater)) and the HTML5 quota system is used instead. So I probably wouldn't fret over this -- perhaps we can remove the hook and leave the implementation as default in our next major.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3391385862
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
@@ -173,6 +160,7 @@ public void onExceededDatabaseQuota(String url, String databaseIdentifier, long
long totalUsedQuota, WebStorage.QuotaUpdater quotaUpdater)
{
LOG.d(LOG_TAG, "onExceededDatabaseQuota estimatedSize: %d currentQuota: %d totalUsedQuota: %d", estimatedSize, currentQuota, totalUsedQuota);
+ long MAX_QUOTA = 100 * 1024 * 1024;
quotaUpdater.updateQuota(MAX_QUOTA);
Review Comment:
> Also worth noting that this whole system is deprecated and apparently no longer used since API 19 (according to https://developer.android.com/reference/android/webkit/WebChromeClient#onExceededDatabaseQuota(java.lang.String,%20java.lang.String,%20long,%20long,%20long,%20android.webkit.WebStorage.QuotaUpdater)) and the HTML5 quota system is used instead. So I probably wouldn't fret over this -- perhaps we can remove the hook and leave the implementation as default in our next major.
Good point. So this code is not used anymore. This could be documented so it's not forget?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] refactor: address warnings in SystemWebChromeClient.java [cordova-android]
Posted by "GitToTheHub (via GitHub)" <gi...@apache.org>.
GitToTheHub commented on code in PR #1951:
URL: https://github.com/apache/cordova-android/pull/1951#discussion_r3419625052
##########
framework/src/org/apache/cordova/engine/SystemWebChromeClient.java:
##########
Review Comment:
Since we found out, that this method is not used anymore since API 19 (Android 4.4) and the HTML5 quota system is used instead, this could be added to the documentation of the method.
Link to Android documentation of this method: https://developer.android.com/reference/android/webkit/WebChromeClient#onExceededDatabaseQuota(java.lang.String,%20java.lang.String,%20long,%20long,%20long,%20android.webkit.WebStorage.QuotaUpdater)
Example:
```javadoc
/**
* Handle database quota exceeded notification.
* This method is no longer called since API 19 (Androd 4.4).
* The HTML5 quota system is used instead.
* The method can be removed in a future release.
* @see https://developer.android.com/reference/android/webkit/WebChromeClient#onExceededDatabaseQuota(java.lang.String,%20java.lang.String,%20long,%20long,%20long,%20android.webkit.WebStorage.QuotaUpdater)
*/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org