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

feat(ios): expose new iOS 14 APIs in Ti.UI.WebView #11834

Merged
merged 22 commits into from Sep 2, 2020

Conversation

vijaysingh-axway
Copy link
Contributor

@build
Copy link
Contributor

build commented Jul 22, 2020

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 5045 tests are passing.
(There are 472 skipped tests not included in that total)

Generated by 🚫 dangerJS against a80e6e9

@build build requested a review from a team July 23, 2020 00:45
@build build added the docs label Jul 23, 2020
@sgtcoolguy
Copy link
Contributor

@vijaysingh-axway formatting is bad for iOS code

apidoc/Titanium/UI/WebView.yml Outdated Show resolved Hide resolved
});

webView.addEventListener('load', function () {
webView.findString('google', function (e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add another test with the search options passed in? (i.e. case sensitive and search by bad case). Test "negative" case: search for something you know you won't find.

Also, this API leaves a bit to be desired. The options for wrapping/searching backwards don't seem very useful if we only get a boolean whether the search was found (rather than say also a position or something). But maybe it matters because it updates some UI state to highlight the result(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In API doc, it is mentioned that "A match found by the search is selected and the page is scrolled to reveal the selection. The completion handler is called after the search completes". But it doesn't scroll and highlight in native as well. Probably beta issue.

tests/Resources/ti.ui.webview.test.js Outdated Show resolved Hide resolved
@tidev tidev deleted a comment from sgtcoolguy Aug 19, 2020
@sgtcoolguy sgtcoolguy self-requested a review August 27, 2020 13:05
Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

Again, my suggestions are not required - and are mainly around docs. I think this would benefit from a. couple examples and then perhaps a unit test that actually did what the proposed Web Archive example showed: saving to a web archive and then loading that in the web view.

Also, consistency around the callback object response by "extending" ErrorResponse in the docs and using the code internally that would make that reality (TiUtils dictionaryWithCode:)

@@ -1114,6 +1156,67 @@ properties:
description: May be undefined.
type: String

---
name: DataCreationResult
summary: The parameter passed to the <Titanium.UI.WebView.createPDF> or <Titanium.UI.WebView.createWebArchive>callback.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
summary: The parameter passed to the <Titanium.UI.WebView.createPDF> or <Titanium.UI.WebView.createWebArchive>callback.
summary: The parameter passed to the <Titanium.UI.WebView.createPDF> or <Titanium.UI.WebView.createWebArchive> callback.

summary: |
Create a PDF document representation from the web page currently displayed in the WebView.
description: |
If the data is written to a file the resulting file is a valid PDF document.
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to include an example in the docs showing how to call this method and then write the blob to a file in the callback.

@@ -1114,6 +1156,67 @@ properties:
description: May be undefined.
type: String

---
name: DataCreationResult
Copy link
Contributor

Choose a reason for hiding this comment

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

Extend ErrorResponse? (I guess we don't set a code property, though?)

May make more sense to align this consistently, though, by doing so.

@"minimumFontSize" : NUMFLOAT([[[[[self webView] webView] configuration] preferences] minimumFontSize]),
@"javaScriptEnabled" : NUMBOOL([[[[[self webView] webView] configuration] preferences] javaScriptEnabled]),
@"javaScriptCanOpenWindowsAutomatically" : NUMBOOL([[[[[self webView] webView] configuration] preferences] javaScriptCanOpenWindowsAutomatically]),
@"minimumFontSize" : NUMFLOAT([[[[self wkWebView] configuration] preferences] minimumFontSize]),
Copy link
Contributor

Choose a reason for hiding this comment

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

All these brackets in all these changes scream out to me to open this in Xcode and use dot notation if possible, i.e.:

Suggested change
@"minimumFontSize" : NUMFLOAT([[[[self wkWebView] configuration] preferences] minimumFontSize]),
@"minimumFontSize" : NUMFLOAT(self.wkWebView.configuration.preferences.minimumFontSize),

Just looks less messy to my eye and easier to follow. Not required (and possibly, not possible!)

ENSURE_TYPE(callback, KrollCallback);

if (![TiUtils isIOSVersionOrGreater:@"14.0"]) {
[callback call:@[ @{ @"success" : NUMBOOL(NO), @"error" : @"Supported on iOS 14+" } ] thisObject:self];
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in the docs, probably want to use the [TiUtils dictionaryWithCode:] for all these callback args (here, and the code below) so we are consistent about the shape of our callback response objects.

- name: createWebArchive
summary: Create WebKit web archive data representing the current web content of the WebView.
description: |
WebKit web archive data represents a snapshot of web content. It can be loaded into a WebView directly,
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unclear to me from the apple docs or looking at our code exactly how this would be used to load into a web view.

Presumably we can save this to a file, great - but how do we then load the file or blob and set it in a web view later? Can we simply provide the Ti.Filesystem.File or Ti.Blob we received as the Ti.UI.WebView.data property and it magically works? If so, perhaps we can provide an example in the docs here and add a unit test which does so?

@ssjsamir ssjsamir self-requested a review August 27, 2020 14:13
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

@vijaysingh-axway When testing the following test case:

var win = Ti.UI.createWindow({

backgroundColor: '#fff'
});
 
var nextWin = Ti.UI.createWindow({
backgroundColor: '#fff'
});
 
var closeButton = Ti.UI.createButton({
top: 50,
title: 'Close'
});
 
closeButton.addEventListener('click', function() {
nextWin.close();
});
 
nextWin.add(closeButton);
 
var nextWebView = Ti.UI.createWebView({
top: 100,
});
 
nextWin.add(nextWebView);
 
 
var btn1 = Ti.UI.createButton({
top: 50,
left: 20,
title: 'Search String (Measure)'
});
 
btn1.addEventListener('click',  function() {
webView.findString('Measure', function(e){ // {'caseSensitive': true, 'backward': false, 'wraps': true},
if (e.success) {
alert('Searched string found!');
} else {
alert('Searched string not found!');
}
});
});
 
win.add(btn1);
 
var btn2 = Ti.UI.createButton({
top: 50,
right: 20,
title: 'Create PDF of webpage'
});
 
btn2.addEventListener('click', function() {
webView.createPdf( function(e) {
nextWebView.data = e.data;
nextWin.open();
});
});
 
win.add(btn2);
 
var btn3 = Ti.UI.createButton({
top: 100,
left: 20,
title: 'Create Web Archive of page'
});
 
btn3.addEventListener('click', function() {
webView.createWebArchive(function(e){
nextWebView.data = e.data;
nextWin.open();
});
});
 
win.add(btn3);
 
var btn4 = Ti.UI.createButton({
top: 100,
right: 20,
title: 'Zoom by factor 2'
});
 
btn4.addEventListener('click', function() {
Ti.API.info('zoom level is ', +webView.zoomLevel);
webView.zoomLevel = 2;
Ti.API.info('zoom level is ', +webView.zoomLevel);
});
 
win.add(btn4);
 
var webView = Ti.UI.createWebView({
url: 'https://appcelerator.com/',
top: 150,
});
 
win.add(webView);
win.open();

I see this error when clicking on 'Create PDF of webpage':

[ERROR] �� �Script Error {

[ERROR] �� � column = 20;

[ERROR] �� � line = 52;

[ERROR] �� � message = "webView.createPdf is not a function. (In 'webView.createPdf(function (e) {\n nextWebView.data = e.data;\n nextWin.open();\n })', 'webView.createPdf' is undefined)";

[ERROR] �� � sourceURL = "file:///Users/samir/Library/Developer/CoreSimulator/Devices/448727D1-8F0B-42DE-85D5-01C0CC011310/data/Containers/Bundle/Application/3642309B-ED82-4E33-AEFC-DCC01D9F0595/anewappp.app/app.js";

[ERROR] �� � stack = "file:///Users/samir/Library/Developer/CoreSimulator/Devices/448727D1-8F0B-42DE-85D5-01C0CC011310/data/Containers/Bundle/Application/3642309B-ED82-4E33-AEFC-DCC01D9F0595/anewappp.app/app.js:52:20";

[ERROR] �� � type = TypeError;

[ERROR] �� �}

Test Environment

MacOS Big Sur: 11.0 Beta 5
Xcode: 12.0 Beta 5
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)
Command line tools 12.0

@vijaysingh-axway
Copy link
Contributor Author

@vijaysingh-axway When testing the following test case:

var win = Ti.UI.createWindow({

backgroundColor: '#fff'
});
 
var nextWin = Ti.UI.createWindow({
backgroundColor: '#fff'
});
 
var closeButton = Ti.UI.createButton({
top: 50,
title: 'Close'
});
 
closeButton.addEventListener('click', function() {
nextWin.close();
});
 
nextWin.add(closeButton);
 
var nextWebView = Ti.UI.createWebView({
top: 100,
});
 
nextWin.add(nextWebView);
 
 
var btn1 = Ti.UI.createButton({
top: 50,
left: 20,
title: 'Search String (Measure)'
});
 
btn1.addEventListener('click',  function() {
webView.findString('Measure', function(e){ // {'caseSensitive': true, 'backward': false, 'wraps': true},
if (e.success) {
alert('Searched string found!');
} else {
alert('Searched string not found!');
}
});
});
 
win.add(btn1);
 
var btn2 = Ti.UI.createButton({
top: 50,
right: 20,
title: 'Create PDF of webpage'
});
 
btn2.addEventListener('click', function() {
webView.createPdf( function(e) {
nextWebView.data = e.data;
nextWin.open();
});
});
 
win.add(btn2);
 
var btn3 = Ti.UI.createButton({
top: 100,
left: 20,
title: 'Create Web Archive of page'
});
 
btn3.addEventListener('click', function() {
webView.createWebArchive(function(e){
nextWebView.data = e.data;
nextWin.open();
});
});
 
win.add(btn3);
 
var btn4 = Ti.UI.createButton({
top: 100,
right: 20,
title: 'Zoom by factor 2'
});
 
btn4.addEventListener('click', function() {
Ti.API.info('zoom level is ', +webView.zoomLevel);
webView.zoomLevel = 2;
Ti.API.info('zoom level is ', +webView.zoomLevel);
});
 
win.add(btn4);
 
var webView = Ti.UI.createWebView({
url: 'https://appcelerator.com/',
top: 150,
});
 
win.add(webView);
win.open();

I see this error when clicking on 'Create PDF of webpage':

[ERROR] �� �Script Error {

[ERROR] �� � column = 20;

[ERROR] �� � line = 52;

[ERROR] �� � message = "webView.createPdf is not a function. (In 'webView.createPdf(function (e) {\n nextWebView.data = e.data;\n nextWin.open();\n })', 'webView.createPdf' is undefined)";

[ERROR] �� � sourceURL = "file:///Users/samir/Library/Developer/CoreSimulator/Devices/448727D1-8F0B-42DE-85D5-01C0CC011310/data/Containers/Bundle/Application/3642309B-ED82-4E33-AEFC-DCC01D9F0595/anewappp.app/app.js";

[ERROR] �� � stack = "file:///Users/samir/Library/Developer/CoreSimulator/Devices/448727D1-8F0B-42DE-85D5-01C0CC011310/data/Containers/Bundle/Application/3642309B-ED82-4E33-AEFC-DCC01D9F0595/anewappp.app/app.js:52:20";

[ERROR] �� � type = TypeError;

[ERROR] �� �}

Test Environment

MacOS Big Sur: 11.0 Beta 5
Xcode: 12.0 Beta 5
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)
Command line tools 12.0

@ssjsamir My bad. After review comment function createPdf renamed to createPDF but I forgot to update test case. Updated test case. Please verify now. Thanks!

@ssjsamir ssjsamir self-requested a review September 1, 2020 15:38
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

@vijaysingh-axway LGTM with the updated test case.

@sgtcoolguy sgtcoolguy merged commit 840b0d2 into tidev:master Sep 2, 2020
@build
Copy link
Contributor

build commented Sep 2, 2020

The backport to 9_3_X failed:

The process 'git' failed with exit code 128

Check the run for full details
To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Check out the target branch
git checkout 9_3_X
# Make sure it's up to date
git pull
# Check out your branch
git checkout -b backport-11834-to-9_3_X
# Apply the commits from the PR
curl -s https://github.com/appcelerator/titanium_mobile/commit/4016622155bbd3c19cc50978ce3eb29a1e92cb22.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/ee68717c7056acebb00bc96a3be108c2ba718841.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/19656c08e8fa8bd72f3e0383b5a0b145adce1d43.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/a277c5e10f9087feaa68593ab37cb9eba220aed5.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/6fb29d34a1f9d313159f2661a71d4d3343bacffa.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/19f236d644bf0856d6ba686e3a7f9f8ffd77ae2b.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/be5a2ab0002353c010580867d2a6392625a24347.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/6a0896be4bbae304dae0fb29f10e55f9b0ad3ea5.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/f803de3c5dd6e9dd4689a87a7181f1949618a732.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/f0d300074c18483a1af7f86a39a8dc1b2f3bdef6.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/3db436cca9cc66af59a72ce784162a11e6c0ff09.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/20e177698bceec025bc6e83147821015afd90466.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/7288e78af370266a944f699b0f46b4cc0db50ada.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/59347ebea7571a11981c67c1c194564fcac58dec.patch | git am -3 --ignore-whitespace
curl -s https://github.com/appcelerator/titanium_mobile/commit/26467be0df637fa42e60825b5c998a4c79874b6d.patch | git am -3 --ignore-whitespace
# Push it to GitHub
git push --set-upstream origin backport-11834-to-9_3_X

Then, create a pull request where the base branch is 9_3_X and the compare/head branch is backport-11834-to-9_3_X.

sgtcoolguy pushed a commit that referenced this pull request Sep 2, 2020
@sgtcoolguy
Copy link
Contributor

manually cherry-picked to 9_3_X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants