feat: add WebView "blockedURLs" property #11842

jquick-axway · 2020-07-24T05:21:17Z

JIRA:
https://jira.appcelerator.org/browse/TIMOB-28030

Summary:

Deprecated WebView "blacklistedURLs" property in favor of new "blockedURLs" property.
Deprecated WebView "blacklisturl" event in favor of new "blockedurl" event.

New API Test:

Build and run the below on Android.
Tap on the "www.apple.com" link.
Verify a dialog is displayed stating "www.apple.com" was blocked.
Tap on the "www.google.com" link.
Verify a dialog is displayed stating "www.google.com" was blocked.
Tap on the "www.axway.com" link.
Verify that the Axway website was loaded. (This requires an Internet connection.)
Repeat steps 1-7 on iOS.

var htmlText =
		'<!DOCTYPE html>\n' +
		'<html>\n' +
		'	<head>\n' +
		'		<meta name="viewport" content="width=device-width, initial-scale=1.0">\n' +
		'	</head>\n' +
		'	<body>\n' +
		'		<p>WebView "blockedURLs" Test</p>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.axway.com">www.axway.com</a>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.apple.com">www.apple.com</a>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.google.com">www.google.com</a>\n' +
		'	</body>\n' +
		'</html>\n';

var window = Ti.UI.createWindow();
var webView = Ti.UI.createWebView({
	html: htmlText,
	blockedURLs: ["www.apple.com", "www.google.com"],
});
webView.addEventListener("blockedurl", function(e) {
	alert("Blocked URL:\n" + e.url);
});
window.add(webView);
window.addEventListener("androidback", function() {
	if (webView.canGoBack()) {
		webView.goBack();
	} else {
		window.close();
	}
});
window.open();

Deprecated API Test:

Build and run the below on Android.
Tap on the "www.apple.com" link.
Verify that a deprecation warning gets logged. (iOS will only log it once.)
Verify a dialog is displayed stating "www.apple.com" was blocked.
Tap on the "www.google.com" link.
Verify a dialog is displayed stating "www.google.com" was blocked.
Tap on the "www.axway.com" link.
Verify that the Axway website was loaded. (This requires an Internet connection.)
Repeat steps 1-8 on iOS.

var htmlText =
		'<!DOCTYPE html>\n' +
		'<html>\n' +
		'	<head>\n' +
		'		<meta name="viewport" content="width=device-width, initial-scale=1.0">\n' +
		'	</head>\n' +
		'	<body>\n' +
		'		<p>WebView "blockedURLs" Test</p>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.axway.com">www.axway.com</a>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.apple.com">www.apple.com</a>\n' +
		'		<br/>\n' +
		'		<br/>\n' +
		'		<a href="https://www.google.com">www.google.com</a>\n' +
		'	</body>\n' +
		'</html>\n';

var window = Ti.UI.createWindow();
var webView = Ti.UI.createWebView({
	html: htmlText,
	blacklistedURLs: ["www.apple.com", "www.google.com"],
});
webView.addEventListener("blacklisturl", function(e) {
	alert("Blocked URL:\n" + e.url);
});
window.add(webView);
window.addEventListener("androidback", function() {
	if (webView.canGoBack()) {
		webView.goBack();
	} else {
		window.close();
	}
});
window.open();

- Deprecated WebView "blacklistedURLs" property in favor of new "blockedURLs" property. - Deprecated WebView "blacklisturl" event in favor of new "blockedurl" event. Fixes TIMOB-28030

build · 2020-07-24T05:48:41Z

	Messages
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖	✅ All tests are passing Nice one! All 4640 tests are passing. (There are 470 skipped tests not included in that total)

Generated by 🚫 dangerJS against c24c2df

ewanharris · 2020-07-27T13:29:26Z

tests/Resources/ti.ui.webview.test.js

@@ -501,6 +502,47 @@ describe('Titanium.UI.WebView', function () {
 		win.open();
 	});

+	it.ios('blockedurl', function (finish) {


Could this be ran on Android too?

Unfortunately no. On Android, we can only "natively" block the links the end-user tapped on. We can't block the URLs that are loaded programmatically. This is something we document.

We can natively detect the URL being navigated to after the HTTP request has been sent by the WebView on Android, but then it's already too late. We may not be able to abort it in time. Especially if a cached HTTP response is received.

ewanharris

Changes look good to me, just a comment on an iOS only test. Thanks for doing this @jquick-axway!

vijaysingh-axway

@jquick-axway iOS changes will work. Code can be updated as per review comment. Thanks!

vijaysingh-axway · 2020-07-27T19:13:42Z

iphone/Classes/TiUIWebView.m

-  // Handle blacklisted URL's
+  // Do not load the URL if it's black-listed.
+  // DEPRECATED: Should use "blockedURLs" property with a "blockedurl" event listener instead.
+  NSString *urlCandidate = navigationAction.request.URL.absoluteString;
  if (_blacklistedURLs != nil && _blacklistedURLs.count > 0) {
-    NSString *urlCandidate = navigationAction.request.URL.absoluteString;
-
-    for (NSString *blackListedURL in _blacklistedURLs) {
-      if ([urlCandidate rangeOfString:blackListedURL options:NSCaseInsensitiveSearch].location != NSNotFound) {
+    for (NSString *blockedURL in _blacklistedURLs) {
+      if ([urlCandidate rangeOfString:blockedURL options:NSCaseInsensitiveSearch].location != NSNotFound) {
        if ([[self proxy] _hasListeners:@"blacklisturl"]) {
          [[self proxy] fireEvent:@"blacklisturl"
                       withObject:@{
                         @"url" : urlCandidate,
                         @"message" : @"Webview did not load blacklisted url."
                       }];
        }
+        decisionHandler(WKNavigationActionPolicyCancel);
+        [self _cleanupLoadingIndicator];
+        return;
+      }
+    }
+  }

+  // Do not load the URL if it's on the block-list.
+  if (_blockedURLs != nil && _blockedURLs.count > 0) {
+    for (NSString *blockedURL in _blockedURLs) {
+      if ([urlCandidate rangeOfString:blockedURL options:NSCaseInsensitiveSearch].location != NSNotFound) {
+        if ([[self proxy] _hasListeners:@"blockedurl"]) {
+          [[self proxy] fireEvent:@"blockedurl"
+                       withObject:@{
+                         @"url" : urlCandidate,
+                         @"message" : @"Webview did not load blocked url."
+                       }];
+        }
        decisionHandler(WKNavigationActionPolicyCancel);
        [self _cleanupLoadingIndicator];
        return;


I would be better to merge 'backlisteurl' event related code snippet inside 'blockedurl' . So above code snippet can be replaced by following-
`
// Do not load the URL if it's on the block-list.
if (_blockedURLs != nil && _blockedURLs.count > 0) {
NSString *urlCandidate = navigationAction.request.URL.absoluteString;
for (NSString *blockedURL in _blockedURLs) {
if ([urlCandidate rangeOfString:blockedURL options:NSCaseInsensitiveSearch].location != NSNotFound) {
if ([[self proxy] _hasListeners:@"blacklisturl"]) {
DEPRECATED_REPLACED(@"UI.WebView.blacklisturl", @"9.2.0", @"UI.WebView.blockedurl");

[[self proxy] fireEvent:@"blacklisturl" withObject:@{ @"url" : urlCandidate, @"message" : @"Webview did not load blacklisted url." }]; } if ([[self proxy] _hasListeners:@"blockedurl"]) { [[self proxy] fireEvent:@"blockedurl" withObject:@{ @"url" : urlCandidate, @"message" : @"Webview did not load blocked url." }]; } decisionHandler(WKNavigationActionPolicyCancel); [self _cleanupLoadingIndicator]; return;`

I separated the 2 on purpose so that we can easily delete the "blacklisted" code in the future.
(Otherwise I wouldn't normally duplicate code like this.)

So, the way it works now is a "blacklisturl" event will only be fired for matches in the blacklistedURLs property... and a "blockedurl" event will only be fired for matches in the blockedURLs property. I purposely treat them separately on both Android and iOS.

Thanks for explaining. Approved!

vijaysingh-axway · 2020-07-27T19:16:31Z

iphone/Classes/TiUIWebView.m

-  for (id blacklistedURL in blacklistedURLs) {
-    ENSURE_TYPE(blacklistedURL, NSString);
+  ENSURE_TYPE(blacklistedURLs, NSArray);
+  for (id nextURL in blacklistedURLs) {
+    ENSURE_TYPE(nextURL, NSString);
  }

  _blacklistedURLs = blacklistedURLs;


Call [self setBlockedURLs_: blacklistedURLs] from here.

ssjsamir

FR Passed, tested using the test case mentioned in the description.

Test Environment

MacOS Big Sur: 11.0 Beta 4
Xcode: 12.0 Beta  4
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.0-master.11""
iphone 8 Sim (14.0 Beta)
API29 Pixel XL

@jquick-axway On iOS does the following message [WARN] �� Ti.UI.WebView.blacklistedURLs DEPRECATED in 9.2.0, in favor of Ti.UI.WebView.blockedURLs only display the first time the app is launched or just it be displayed only once every-time a blocked URL is clicked?

jquick-axway · 2020-08-18T16:13:04Z

@ssjsamir,

On iOS, it logs a deprecation warning every time you get/set the "blacklistedURLs" property.
On Android, it logs a deprecation warning when you tap on a blacklisted link.

Yes, it's inconsistent, but I didn't think it was worth reworking the Android code to make it behave more like iOS.

feat: add WebView blockedURLs property

d7e10b3

- Deprecated WebView "blacklistedURLs" property in favor of new "blockedURLs" property. - Deprecated WebView "blacklisturl" event in favor of new "blockedurl" event. Fixes TIMOB-28030

jquick-axway added ios android improvement no tests labels Jul 24, 2020

jquick-axway added this to the 9.2.0 milestone Jul 24, 2020

jquick-axway requested review from sgtcoolguy, garymathews and ewanharris July 24, 2020 05:21

build requested review from a team July 24, 2020 05:48

build added the docs label Jul 24, 2020

jquick-axway added needs tests 🚨 and removed no tests labels Jul 24, 2020

jquick-axway added 2 commits July 24, 2020 13:58

Merge branch 'master' into TIMOB-28030

02bbb49

test: add WebView blockedURLs unit tests

c24c2df

build removed the needs tests 🚨 label Jul 24, 2020

ewanharris reviewed Jul 27, 2020

View reviewed changes

ewanharris approved these changes Jul 27, 2020

View reviewed changes

vijaysingh-axway reviewed Jul 27, 2020

View reviewed changes

vijaysingh-axway approved these changes Jul 27, 2020

View reviewed changes

garymathews approved these changes Jul 27, 2020

View reviewed changes

jquick-axway mentioned this pull request Jul 27, 2020

feat(9_3_X): add WebView "blockedURLs" property #11848

Merged

jquick-axway added the in-qe-testing 🕵 label Jul 28, 2020

jquick-axway added backport 9_3_X and removed backport 9_3_X labels Aug 4, 2020

ssjsamir self-requested a review August 17, 2020 13:17

ssjsamir approved these changes Aug 18, 2020

View reviewed changes

sgtcoolguy added the backport 9_3_X label Aug 21, 2020

sgtcoolguy merged commit c1c4af2 into tidev:master Aug 21, 2020

build mentioned this pull request Aug 21, 2020

[Backport 9_3_X] feat: add WebView "blockedURLs" property #11940

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add WebView "blockedURLs" property #11842

feat: add WebView "blockedURLs" property #11842

jquick-axway commented Jul 24, 2020

build commented Jul 24, 2020 •

edited

ewanharris Jul 27, 2020

jquick-axway Jul 27, 2020

ewanharris left a comment

vijaysingh-axway left a comment

vijaysingh-axway Jul 27, 2020 •

edited

jquick-axway Jul 27, 2020

vijaysingh-axway Jul 27, 2020

vijaysingh-axway Jul 27, 2020

ssjsamir left a comment

jquick-axway commented Aug 18, 2020

feat: add WebView "blockedURLs" property #11842

feat: add WebView "blockedURLs" property #11842

Conversation

jquick-axway commented Jul 24, 2020

build commented Jul 24, 2020 • edited

ewanharris Jul 27, 2020

Choose a reason for hiding this comment

jquick-axway Jul 27, 2020

Choose a reason for hiding this comment

ewanharris left a comment

Choose a reason for hiding this comment

vijaysingh-axway left a comment

Choose a reason for hiding this comment

vijaysingh-axway Jul 27, 2020 • edited

Choose a reason for hiding this comment

jquick-axway Jul 27, 2020

Choose a reason for hiding this comment

vijaysingh-axway Jul 27, 2020

Choose a reason for hiding this comment

vijaysingh-axway Jul 27, 2020

Choose a reason for hiding this comment

ssjsamir left a comment

Choose a reason for hiding this comment

jquick-axway commented Aug 18, 2020

build commented Jul 24, 2020 •

edited

vijaysingh-axway Jul 27, 2020 •

edited