fix(ios): allow changing WebView read access when loading local file #10976

vijaysingh-axway · 2019-06-18T18:30:42Z

https://jira.appcelerator.org/browse/TIMOB-27159

build · 2019-06-18T18:32:29Z

	Fails
🚫	Tests have failed, see below for more information.

	Warnings
⚠️	Commit 823f5f5861dd3ca752c5576065a86fb0ce1cec0a has a message "fix(ios): Added new method to load local file" giving 1 errors: subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️	Commit 5fd6e30f576b9439f85e793b6d346bb406259b37 has a message "fix(ios): Updated doc" giving 1 errors: subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️	Commit 2347659b10c8fd43d7b0ba8e51d0408258aa9733 has a message "fix(iOS) : Added property assetsDirectory for ios to load local file" giving 2 errors: subject may not be empty type may not be empty
⚠️	Commit 44984e2b9470db2acd06fb87dbdccca23415acaa has a message "fix(iOS): Updated doc" giving 2 errors: scope must be lower-case subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️	Commit e180fbfcdfd1f63500b9a465c832a1b17387b868 has a message "fix(ios): Updated doc" giving 1 errors: subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️	Commit bc2975004a45c7382952310b91485156d451aff5 has a message "fix(iOS): Removed accidentally commited file timob3123.html" giving 2 errors: scope must be lower-case subject must not be sentence-case, start-case, pascal-case, upper-case

	Messages
📖	💾 Here's the generated SDK zipfile.
📖	❌ 5 tests have failed There are 5 tests failing and 472 skipped out of 4371 total tests.
📖	🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

Tests:

Classname	Name	Time	Error
ios.os	#hostname() returns Ti.Platform.address value	0	Error: expected '' to equal undefined fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/node_modules/should/cjs/should.js:275:23 value@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/node_modules/should/cjs/should.js:356:23 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/os.test.js:123:31 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4376:41 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4763:17 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4840:23 next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4688:20 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4698:15 next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4636:30 file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4665:13 timeslice@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:5764:29
ios.Titanium.UI.WebView	#assetsDirectory	30.001	Error: timeout of 30000ms exceeded file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4326:27
ios.Titanium.UI.WebView	html-script-tag	10.003	Error: timeout of 10000ms exceeded file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4290:32
ios.Titanium.UI.WebView	should handle file URLs with spaces in path - TIMOB-18765	30	Error: timeout of 30000ms exceeded file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4326:27
ios.Titanium.UI.WebView	#evalJS(string, function) - async variant	30	Error: timeout of 30000ms exceeded file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/F2554B3A-BF61-4763-A475-FE479C1ABFEE/mocha.app/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against e555f7f

hansemannn · 2019-06-18T19:59:04Z

apidoc/Titanium/UI/WebView.yml

@@ -170,6 +170,22 @@ methods:
    since: {android: "8.1.0", iphone: "2.0.0", ipad: "2.0.0"}
    platforms: [android, iphone, ipad]

+  - name: loadFile


Is this for loading html files? Or for other resources?

All html files should be handled via "url" which is already pretty smart handling different types of urls. In additions, assets should be loaded through the bridge between the WKWebView and the web-view handler.

janvennemann · 2019-06-18T20:38:05Z

@vijaysingh-axway does this need parity with Android?

vijaysingh-axway · 2019-06-18T20:42:29Z

@vijaysingh-axway does this need parity with Android?

Android do not have anything similar. @jquick-axway has mentioned in comment of this ticket.

jquick-axway · 2019-06-19T01:06:38Z

When setting this to a local HTML file, wouldn't everyone want to set allowingReadAccessToURL to the HTML file's directory?

I don't see anyone ever setting it to a different directory. That doesn't seem practical.

Also, I can seeing this being as a non-stop tech-support issue for us. We would end up telling everyone to call the loadFile() method and pass it the HTML file's directory. Also, only being able to do this via a loadFile() method isn't Alloy friendly where we want to use properties instead.

I'm leaning toward us automatically setting allowingReadAccessToURL to the local HTML's file directory. This makes it backward compatible, avoids tech-support issues, and has feature parity between platforms.

Is there any reason not to set this automatically?

janvennemann · 2019-06-19T12:05:41Z

@jquick-axway The ticket author mentioned it would be nice to change allowingReadAccessToURL so that different HTML files in the cache dir could reuse shared resources like css that are already in the app's resources folder.

However, i think it would be best to copy the local CSS files to the cache dir and then set allowingReadAccessToURL automatically. This is basically what you are doing in your example posted in the ticket, correct @vijaysingh-axway? I was also wondering if this could be integrated in url property setter so we don't need a new method?

sgtcoolguy · 2019-06-19T14:19:19Z

So my 2 cents here: I really don't like the idea of adding some epochal extra method to web view that is both iOS-specific *and specific to a particular use case of files in the app cache.

As @jquick-axway @hansemannn and @janvennemann are alluding to, we should try and "do the right thing" under the hood in our existing APIs (like the url property).

vijaysingh-axway · 2019-06-19T18:32:44Z

Agreed. What if I modify the setUrl to take extra optional parameter 'readAccessUrl' similar to setHtml
and modify implementation of internal function loadLocalURL . If 'readAccessUrl' is available we can use that value, otherwise default value which will be directory of passed 'url'. So developer need not to modify their implementation until they really need e.g. test case given in ticket. In android, we can modify implementation of 'setUrl' to ignore extra parameter, if it required.

jquick-axway · 2019-06-19T19:44:28Z

The default handling of all other WebViews is to use the local HTML file's directory as the base directory. There is no equivalent to allowingReadAccessToURL on other platforms.

If a developer wants the base directory to be different, then we already have an existing solution. They can call the setHtml(htmlString, { baseUrl: path }) method, where they can read in the local HTML file as a string and pass it to that method. For that API, setting a base directory makes sense since the WebView needs to be told what paths should relative to since it did not load HTML from file itself.

hansemannn · 2019-06-19T20:24:12Z

I'd extend the url setter and add an iOS-only property assetsDirectory with defaults to the current setting. Internally, you can just set a helper property and re-use it in the url setter. You just need to ensure that the keySequence lists the assetsDirectory before url, so the setters are always called after each other. Pretty light change actually

jquick-axway · 2019-06-19T20:44:29Z

Guys. We're waaayyy over thinking this.
Setting the base directory to the local HTML file is enough.

Also, if you look at the ticket's attached sample code (Resources.zip), you'll see that the HTML file is loading the CSS file via a relative path like this "../css/test.css". Meaning that the HTML is assuming that the base directory is the same location as the HTML file. We just need to restore the old behavior. We've never offered the ability to set the base directory to something different than the HTML file's directory before. So, we're losing nothing.

Let's not waste time implementing a new feature that no one is going to use.

vijaysingh-axway · 2019-06-19T22:16:20Z

@jquick-axway As we discussed, when there is any directory structure like ../Cache/html/example.html , ../Cache/css/test.css if we use default access directory ../Cache/html/, it will not whitelist ../Cache/css/test.css for webkit. For whitelisting ../Cache/css/test.css we have to whitelist ../Cache/ directory. I agree with Hans's proposal to make iOS specific property.

janvennemann · 2019-06-24T07:05:12Z

I think this is a good middle ground solution between getting parity of Android and iOS and having too much platform specific APIs. With this change loading local resources that are in the same directory or in a sub directory as the html works out of the box on both platforms. Referring resources from parent directories like the ../css/test.css for example will need setting the property on iOS.

@vijaysingh-axway I think we should add a few notes to the docs explaining when the assetDirectory is required and what its default value is.

janvennemann · 2019-06-24T07:08:53Z

apidoc/Titanium/UI/WebView.yml

@@ -657,6 +657,15 @@ properties:
    since: "3.0.0"
    default: false

+  - name: assetsDirectory
+    summary: Path of file or directory to allow read access.


Suggested change

summary: Path of file or directory to allow read access.

summary: Path of file or directory to allow read access by the WebView.

janvennemann · 2019-06-24T07:31:03Z

apidoc/Titanium/UI/WebView.yml

+  - name: assetsDirectory
+    summary: Path of file or directory to allow read access.
+    description: |
+        If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a directory, 


Suggested change

If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a directory,

Use this to change the resources the web view has access to when loading the content of a local file. By default

the web view only has access to files inside the same directory as the loaded file. To reference resources from

other directories (e.g. a parent directory) change this property accordingly.

If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a

directory, files inside that directory may be loaded.

janvennemann · 2019-06-24T07:31:26Z

apidoc/Titanium/UI/WebView.yml

+    summary: Path of file or directory to allow read access.
+    description: |
+        If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a directory, 
+        files inside that directory may be loaded. This property should be set with [url](Titanium.UI.WebView.url) property or before it. 


Suggested change

files inside that directory may be loaded. This property should be set with [url](Titanium.UI.WebView.url) property or before it.

This property needs to be set before [url](Titanium.UI.WebView.url) is assigned to a local file.

janvennemann

LGTM!

janvennemann · 2019-06-26T10:30:40Z

@vijaysingh-axway Please add a unit test for the new property, thanks!

jigneshkasundra · 2019-07-24T05:51:15Z

hello,

can you please review and confirm is it related to same ? file stored under applicationDataDirectory not loading into webview

https://jira.appcelerator.org/browse/AC-6319

keerthi1032 · 2019-08-05T18:04:27Z

FR passed. WebView works as expected .loading HTML files . Waiting for 8.1.0 GA to merge.

Test Environment:
Operating System
Name = Mac OS X
Version = 10.14.5
Architecture = 64bit
Node.js
Node.js Version = 10.16.0
npm Version = 6.9.0
Titanium CLI
CLI Version = 5.2.1
Titanium SDK
SDK Version = local sdk 8.2.0.v20190724052958
Device -iphone x iOS 11
Simulator -iPhone 6s iOS 12.4
Studio =5.1.3.201907112159
Cli -7.1.0.-24

janvennemann · 2019-08-13T19:04:07Z

@vijaysingh-axway can you please add the missing unit tests before this gets merged? Also there seems to be a related issue with setHtml, see AC-6319.

sgtcoolguy · 2019-08-29T19:57:47Z

@vijaysingh-axway This is repeatedly failing some of the WebView unit tests due to a timeout. I don't think we can merge this until that is resolved.

sgtcoolguy

4 WebView tests are timing out with this PR. They need to be working for this to be ready to merge.

vijaysingh-axway · 2019-08-29T21:56:41Z

Looks like iOS 13 broken WKWebView's API - (nullable WKNavigation *)loadFileURL:(NSURL *)URL allowingReadAccessToURL:(NSURL *)readAccessURL. https://forums.developer.apple.com/thread/120566
In iOS 12 it is working fine. Looking in this.

vijaysingh-axway · 2020-01-14T00:39:34Z

Closing in favor of PR #11431 .

fix(ios): Added new method to load local file

823f5f5

vijaysingh-axway requested review from jquick-axway and janvennemann and removed request for jquick-axway June 18, 2019 18:30

build added this to the 8.1.0 milestone Jun 18, 2019

build requested review from a team June 18, 2019 18:32

build added docs ios needs tests 🚨 labels Jun 18, 2019

fix(ios): Updated doc

5fd6e30

hansemannn reviewed Jun 18, 2019

View reviewed changes

sgtcoolguy modified the milestones: 8.1.0, 8.2.0 Jun 19, 2019

vijaysingh-axway added 2 commits June 19, 2019 17:28

fix(iOS) : Added property assetsDirectory for ios to load local file

2347659

fix(iOS): Updated doc

44984e2

vijaysingh-axway changed the title ~~fix(ios): Added new method to load local file~~ chore(ios):Added property assetsDirectory for ios to load local file Jun 20, 2019

janvennemann requested changes Jun 24, 2019

View reviewed changes

vijaysingh-axway and others added 2 commits June 25, 2019 16:02

fix(iOS): Removed accidentally commited file timob3123.html

bc29750

docs: fix formatting

62a19d0

janvennemann approved these changes Jun 26, 2019

View reviewed changes

janvennemann added the in-qe-testing 🕵 label Jul 24, 2019

Merge branch 'master' into AC-6247

632e524

Merge branch 'master' into AC-6247

b989061

keerthi1032 and others added 2 commits August 16, 2019 09:27

Merge branch 'master' into AC-6247

f7baba6

fix(ios): added unit test

b9f53c4

build removed the needs tests 🚨 label Aug 16, 2019

keerthi1032 and others added 5 commits August 19, 2019 11:23

Merge branch 'master' into AC-6247

fd14327

Merge branch 'master' into AC-6247

456b8b3

Merge branch 'master' into AC-6247

c552ac3

Merge branch 'master' into AC-6247

694d10c

Merge branch 'master' into AC-6247

e555f7f

sgtcoolguy self-requested a review August 29, 2019 19:59

sgtcoolguy suggested changes Aug 29, 2019

View reviewed changes

sgtcoolguy modified the milestones: 8.2.0, 8.2.1 Sep 5, 2019

sgtcoolguy modified the milestones: 8.2.1, 9.0.0 Dec 10, 2019

sgtcoolguy added bug and removed in-qe-testing 🕵 labels Dec 10, 2019

vijaysingh-axway closed this Jan 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ios): allow changing WebView read access when loading local file #10976

fix(ios): allow changing WebView read access when loading local file #10976

vijaysingh-axway commented Jun 18, 2019 •

edited

build commented Jun 18, 2019 •

edited

hansemannn Jun 18, 2019

vijaysingh-axway Jun 18, 2019

hansemannn Jun 18, 2019

janvennemann commented Jun 18, 2019

vijaysingh-axway commented Jun 18, 2019

jquick-axway commented Jun 19, 2019

janvennemann commented Jun 19, 2019

sgtcoolguy commented Jun 19, 2019

vijaysingh-axway commented Jun 19, 2019 •

edited

jquick-axway commented Jun 19, 2019

hansemannn commented Jun 19, 2019

jquick-axway commented Jun 19, 2019

vijaysingh-axway commented Jun 19, 2019

janvennemann commented Jun 24, 2019 •

edited

janvennemann Jun 24, 2019

janvennemann Jun 24, 2019

janvennemann Jun 24, 2019

janvennemann left a comment

janvennemann commented Jun 26, 2019

jigneshkasundra commented Jul 24, 2019

keerthi1032 commented Aug 5, 2019

janvennemann commented Aug 13, 2019

sgtcoolguy commented Aug 29, 2019

sgtcoolguy left a comment

vijaysingh-axway commented Aug 29, 2019

vijaysingh-axway commented Jan 14, 2020

	summary: Path of file or directory to allow read access.
	summary: Path of file or directory to allow read access by the WebView.

-        If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a directory,
+        Use this to change the resources the web view has access to when loading the content of a local file. By default
+        the web view only has access to files inside the same directory as the loaded file. To reference resources from
+        other directories (e.g. a parent directory) change this property accordingly.
+        If assetsDirectory references a single file, only that file may be loaded. If assetsDirectory references a
+        directory, files inside that directory may be loaded.

	files inside that directory may be loaded. This property should be set with [url](Titanium.UI.WebView.url) property or before it.

	This property needs to be set before [url](Titanium.UI.WebView.url) is assigned to a local file.

fix(ios): allow changing WebView read access when loading local file #10976

fix(ios): allow changing WebView read access when loading local file #10976

Conversation

vijaysingh-axway commented Jun 18, 2019 • edited

build commented Jun 18, 2019 • edited

Tests:

hansemannn Jun 18, 2019

Choose a reason for hiding this comment

vijaysingh-axway Jun 18, 2019

Choose a reason for hiding this comment

hansemannn Jun 18, 2019

Choose a reason for hiding this comment

janvennemann commented Jun 18, 2019

vijaysingh-axway commented Jun 18, 2019

jquick-axway commented Jun 19, 2019

janvennemann commented Jun 19, 2019

sgtcoolguy commented Jun 19, 2019

vijaysingh-axway commented Jun 19, 2019 • edited

jquick-axway commented Jun 19, 2019

hansemannn commented Jun 19, 2019

jquick-axway commented Jun 19, 2019

vijaysingh-axway commented Jun 19, 2019

janvennemann commented Jun 24, 2019 • edited

janvennemann Jun 24, 2019

Choose a reason for hiding this comment

janvennemann Jun 24, 2019

Choose a reason for hiding this comment

janvennemann Jun 24, 2019

Choose a reason for hiding this comment

janvennemann left a comment

Choose a reason for hiding this comment

janvennemann commented Jun 26, 2019

jigneshkasundra commented Jul 24, 2019

keerthi1032 commented Aug 5, 2019

janvennemann commented Aug 13, 2019

sgtcoolguy commented Aug 29, 2019

sgtcoolguy left a comment

Choose a reason for hiding this comment

vijaysingh-axway commented Aug 29, 2019

vijaysingh-axway commented Jan 14, 2020

vijaysingh-axway commented Jun 18, 2019 •

edited

build commented Jun 18, 2019 •

edited

vijaysingh-axway commented Jun 19, 2019 •

edited

janvennemann commented Jun 24, 2019 •

edited