feat(allow-list)!: integrate and refactor core plugin #1138

erisu · 2020-12-01T08:10:12Z

Motivation, Context & Description

Integrate the Allow List core plugin to platform.

Testing

npm t
cordova create
cordova platform add
cordova build android

Run in emulator though Android Studio.

Must remove the original plugin that is configured with new projects.

Checklist

I've run the tests to see all new and existing tests pass
I've updated the documentation if necessary

NiklasMerz

Code LGTM 👍

dpogue

This looks okay to me

dpogue · 2020-12-02T04:55:03Z

framework/src/org/apache/cordova/AllowListPlugin.java

+                boolean external = (xml.getAttributeValue(null, "launch-external") != null);
+                if (origin != null) {
+                    if (external) {
+                        LOG.w(LOG_TAG, "Found <access launch-external> within config.xml. Please use <allow-intent> instead.");


Now seems like a good time to drop this deprecated <access launch-external> support?

Yes, we can remove this now too... I didn't carefully read to notice that this deprecated code existed. I will go ahead and delete this.

@dpogue The launch-external has been removed: 4fc48ff?diff=split&w=1

I also moved the String subdomains into the else statement where it is used.

codecov-io · 2020-12-02T06:52:13Z

Codecov Report

Merging #1138 (4fc48ff) into master (97e2d15) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1138   +/-   ##
=======================================
  Coverage   71.19%   71.19%           
=======================================
  Files          21       21           
  Lines        1739     1739           
=======================================
  Hits         1238     1238           
  Misses        501      501

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97e2d15...4fc48ff. Read the comment docs.

breautek

lgtm

Whitelist class rename to AllowList sounds like a breaking change to me, which means this should be targeted for cordova-android@10, correct?

erisu · 2020-12-04T02:48:48Z

Yes, because the core's original classname was renamed, this is a major change.

NiklasMerz · 2021-05-12T06:05:11Z

Should we resolve the conflicts and move forward with this? Especially as we should solve #1217 after merging this PR.

…::handleStartTag

codecov-commenter · 2021-07-01T08:19:26Z

Codecov Report

Merging #1138 (8c3f59c) into master (0f13f4a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1138   +/-   ##
=======================================
  Coverage   71.90%   71.90%           
=======================================
  Files          21       21           
  Lines        1694     1694           
=======================================
  Hits         1218     1218           
  Misses        476      476

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f13f4a...8c3f59c. Read the comment docs.

test/androidx/app/src/main/assets/www/whitelist/index.html

test/android/app/src/main/assets/www/index.html

PieterVanPoyer

Awesome effort. Great.

NiklasMerz · 2022-02-26T20:54:42Z

@dalyboub Please go to Stack Overflow or Slack for questions.

Issues and pullrequests are not for questions.

rmoehn · 2022-04-21T08:24:06Z

Congratulations, you've broken my code. Granted, using the word ‘whitelist’ might make some people feel bad. But what about all the developer hours wasted by having to deal with a breaking change? Does this not factor into your ethics?

breautek · 2022-04-21T09:09:48Z

Congratulations, you've broken my code. Granted, using the word ‘whitelist’ might make some people feel bad. But what about all the developer hours wasted by having to deal with a breaking change? Does this not factor into your ethics?

Apache uses a Code of Conduct to guide decisions such as these. More importantly we had community users raise the issue and other community members volunteer their time to solve their complaints.

We also strive not to include breaking changes. However, sometimes it is necessary. We reserve breaking changes to major version updates, so that by default, breaking changes aren't automatically rolled into your projects.

As with any library or framework using the semver version model, it would be expected that major version upgrades requires some foresight and may require refactoring on our users end. Breaking changes are generally listed in our changelog.

If you want to discuss Cordova's strategies for releases or provide advice on how we can improve the release process that can be done via our Mailing List. A closed PR is not the place for that kind of discussion. For that reason I'm locking this thread.

erisu added the enhancement label Dec 1, 2020

erisu requested a review from dpogue December 1, 2020 08:10

NiklasMerz approved these changes Dec 1, 2020

View reviewed changes

dpogue reviewed Dec 2, 2020

View reviewed changes

erisu mentioned this pull request Dec 3, 2020

breaking: deprecate plugin apache/cordova-plugin-whitelist#59

Merged

1 task

breautek approved these changes Dec 3, 2020

View reviewed changes

breautek added this to the 10.0.0 milestone Dec 3, 2020

erisu requested a review from dpogue December 4, 2020 02:49

mosabab mentioned this pull request Apr 9, 2021

Remove deprecated plugin-whitelist apache/cordova-app-hello-world#68

Closed

5 tasks

jcesarmobile mentioned this pull request Apr 27, 2021

Port Whitelist naming & terms to Allowlist (or alternative) apache/cordova-plugin-whitelist#57

Closed

jcesarmobile mentioned this pull request May 11, 2021

Application does not initialize correctly on 10.x nightly/dev master #1217

Closed

3 tasks

PieterVanPoyer mentioned this pull request Jun 17, 2021

Feat/allowlist master is merged #1244

Closed

5 tasks

erisu added 4 commits July 1, 2021 17:17

refactor: naming to allow list

c02d0b5

feat(allow-list): integrate core plugin

243d2e0

refactor(allow-list): cleanup

b15e944

refactor: drop deprecated launch-external case for CustomConfigParser…

8c3f59c

…::handleStartTag

erisu force-pushed the feat/allowlist branch from 4fc48ff to 8c3f59c Compare July 1, 2021 08:17

PieterVanPoyer reviewed Jul 1, 2021

View reviewed changes

test/androidx/app/src/main/assets/www/whitelist/index.html Show resolved Hide resolved

test/android/app/src/main/assets/www/index.html Outdated Show resolved Hide resolved

fix: apply review comments

8e8cbbf

erisu requested a review from PieterVanPoyer July 1, 2021 10:20

PieterVanPoyer approved these changes Jul 1, 2021

View reviewed changes

erisu merged commit 015db81 into apache:master Jul 2, 2021

erisu deleted the feat/allowlist branch July 2, 2021 02:52

erisu changed the title ~~feat(allow-list): integrate and refactor core plugin~~ feat(allow-list)!: integrate and refactor core plugin Jul 2, 2021

NiklasMerz mentioned this pull request Jul 14, 2021

fix: add WebViewAssetloader to default allow list #1275

Merged

5 tasks

Luro91 mentioned this pull request Jul 21, 2021

Cordova Android 10.0.0 not compiling - cannot find symbol "import org.apache.cordova.Whitelist;" apache/cordova-plugin-file-transfer#306

Closed

3 tasks

This comment was marked as off-topic.

Sign in to view

apache locked as resolved and limited conversation to collaborators Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(allow-list)!: integrate and refactor core plugin #1138

feat(allow-list)!: integrate and refactor core plugin #1138

erisu commented Dec 1, 2020

NiklasMerz left a comment

dpogue left a comment

dpogue Dec 2, 2020

erisu Dec 2, 2020 •

edited

Loading

erisu Dec 2, 2020 •

edited

Loading

codecov-io commented Dec 2, 2020 •

edited

Loading

breautek left a comment

erisu commented Dec 4, 2020

NiklasMerz commented May 12, 2021

codecov-commenter commented Jul 1, 2021 •

edited

Loading

PieterVanPoyer left a comment

This comment was marked as off-topic.

NiklasMerz commented Feb 26, 2022

rmoehn commented Apr 21, 2022

breautek commented Apr 21, 2022 •

edited

Loading

feat(allow-list)!: integrate and refactor core plugin #1138

feat(allow-list)!: integrate and refactor core plugin #1138

Conversation

erisu commented Dec 1, 2020

Motivation, Context & Description

Testing

Checklist

NiklasMerz left a comment

Choose a reason for hiding this comment

dpogue left a comment

Choose a reason for hiding this comment

dpogue Dec 2, 2020

Choose a reason for hiding this comment

erisu Dec 2, 2020 • edited Loading

Choose a reason for hiding this comment

erisu Dec 2, 2020 • edited Loading

Choose a reason for hiding this comment

codecov-io commented Dec 2, 2020 • edited Loading

Codecov Report

breautek left a comment

Choose a reason for hiding this comment

erisu commented Dec 4, 2020

NiklasMerz commented May 12, 2021

codecov-commenter commented Jul 1, 2021 • edited Loading

Codecov Report

PieterVanPoyer left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

NiklasMerz commented Feb 26, 2022

rmoehn commented Apr 21, 2022

breautek commented Apr 21, 2022 • edited Loading

erisu Dec 2, 2020 •

edited

Loading

erisu Dec 2, 2020 •

edited

Loading

codecov-io commented Dec 2, 2020 •

edited

Loading

codecov-commenter commented Jul 1, 2021 •

edited

Loading

breautek commented Apr 21, 2022 •

edited

Loading