Also, cue *sad violin*: the Xcode 6.2 beta SDK (iOS 8.2) does *not* include the WKWebView loadFileURL:readAccessURL: method.
On Wed, Nov 19, 2014 at 9:19 AM, Shazron <shaz...@gmail.com> wrote: > I commented on Ian's comment on CB-8032: > https://issues.apache.org/jira/browse/CB-8032?focusedCommentId=14216403&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14216403 > > My goal was to have a loose coupling of this functionality (CDVFile need > not know about LocalWebServer at all) -- and Ian's comment of this change > is that would impact all CDVFileSystem instances makes this not an ideal > solution, what if you have two Cordova WebView instances, etc. > > My revised proposal requires CDVFileSystem to have a delegate that can be > set. Any class can set it to override the nativeURL behaviour, and > CDVFileSystem will call this method in the delegate if available. It > achieves the same goal without the potentially undefined behaviour of an > Obj-C Category. > > The LocalWebServer gets the currently installed File plugin, enumerates > all available filesystems, and sets this delegate on each, to its own > implementation. > > Tony - I think this is approach is cleaner, and is more maintainable. > > On Wed, Nov 19, 2014 at 6:20 AM, Ian Clelland <iclell...@chromium.org> > wrote: > >> On Tue Nov 18 2014 at 2:00:34 PM Jesse <purplecabb...@gmail.com> wrote: >> >> > Shaz's solution has less impact and seems more elegant. >> > >> > // if ([self respondsToSelector:@selector(nativeFullPath:)]) { >> > >> > If no-one ( generically ) has provided the nativeFullPath method, then >> use >> > it as is, otherwise call it. >> > No need for any (direct) dependency between File + LocalServer. >> > >> >> My first instinct, looking at the code, was that it was wrong, exactly >> because there *is* a dependency. You don't normally add code to a base >> class to change its behaviour when there is a category defined on it. >> Normally that goes the other way: when you add a category to a base class, >> all of the code that's relevant to that category is *in the category*, and >> the base class needs to know nothing at all about it, including its >> existence. >> >> As I said in the PR, it may be that this really is the cleanest and best >> way to go forward with this; I just wanted to have this discussion and >> figure it out with the community before committing to any >> hard-to-change-later technical debt. It does become an API surface, and we >> will have to maintain whatever we adopt. >> >> Ian >> >> >> > >> > @purplecabbage >> > risingj.com >> > >> > On Tue, Nov 18, 2014 at 10:42 AM, Andrew Grieve <agri...@chromium.org> >> > wrote: >> > >> > > Having the localserver plugin add behaviour to file plugin feels like >> the >> > > dependency is in the wrong direction to me. >> > > >> > > How about having CDVFile.m do something like: >> > > >> > > CDVPlugin* p = [commandDelegate getCommandInstance:@"LocalServer"]; >> > > if (p != nil) { >> > > nativeURL = [p transformURL:nativeURL]; // do some local >> declaration to >> > > make this not complain about unrecognized selector >> > > } >> > > >> > > Would probably also need an "untransformURL" to go the other >> direction as >> > > well. >> > > >> > > On Tue, Nov 18, 2014 at 12:05 AM, Shazron <shaz...@gmail.com> wrote: >> > > >> > > > Filed https://issues.apache.org/jira/browse/CB-8032 with pull >> request >> > > > included. >> > > > >> > > > On Mon, Nov 17, 2014 at 2:47 PM, Shazron <shaz...@gmail.com> wrote: >> > > > >> > > > > Sorry I should have looked into the File API code first (no >> > JavaScript >> > > > > changes, that would not work). >> > > > > >> > > > > Essentially I need to "override" this line from my plugin: >> > > > > >> > > > >> > > https://github.com/apache/cordova-plugin-file/blob/ >> > 82f803ea0d61cde051dcffd27b21dc0ed92a0fdf/src/ios/ >> > CDVAssetLibraryFilesystem.m#L74 >> > > > > (plus the CDVLocalFileSystem equivalent). >> > > > > >> > > > > Since Obj-C categories can't really "override" methods (behavior >> > > > > undefined), and I don't want to do some runtime swap >> implementation >> > > > voodoo, >> > > > > I would replace the line above with something like: >> > > > > >> > > > > NSString* nativeURL = [NSString stringWithFormat:@ >> > > > > "assets-library:/%@",fullPath]; >> > > > > if ([self respondsToSelector:@selector(nativeFullPath:)]) { // >> some >> > > > > unique selector name perhaps >> > > > > nativeURL = [self nativeFullPath:fullPath]; // this code >> won't >> > > > > compile, pseudo-code for now. Will call my category method >> defined in >> > > my >> > > > > plugin for CDVAssetLibraryFileSystem >> > > > > } >> > > > > dirEntry[@"nativeURL"] = nativeURL; >> > > > > >> > > > > Backwards compatible. >> > > > > >> > > > > >> > > > > On Mon, Nov 17, 2014 at 1:44 PM, Shazron <shaz...@gmail.com> >> wrote: >> > > > > >> > > > >> Local Web Server Checklist: >> > > > >> 1. We have random port usage >> > > > >> 2. We have the token/cookie check >> > > > >> 3. We have the localhost check >> > > > >> 4. The app is now installed under http://localhost:RANDOM_PORT >> /www/ >> > > > >> >> > > > >> It'll be easy (relatively) to add support for: >> > > > >> http://localhost:RANDOM_PORT/asset-library/ >> > > > >> http://localhost:RANDOM_PORT/file-system/ >> > > > >> >> > > > >> The only issue now is changing FileEntry.toURL(). I'm thinking of >> > some >> > > > >> runtime 'magic' in the local web server where it detects the File >> > > > plugin, >> > > > >> and change the implementation of FileEntry.toURL() (or through >> > > injecting >> > > > >> JavaScript, probably easier). >> > > > >> >> > > > >> >> > > > >> On Wed, Oct 29, 2014 at 5:11 PM, Andrew Grieve < >> > agri...@chromium.org> >> > > > >> wrote: >> > > > >> >> > > > >>> We could restrict access to the webserver by stuffing a cookie >> into >> > > the >> > > > >>> webview with an access token, then have the server just 500 on >> any >> > > > >>> request >> > > > >>> missing the cookie. We should also be able to restrict external >> > > > requests >> > > > >>> just by listening on 127.0.0.1 instead of 0.0.0.0 (doesn't look >> > > > >>> like GCDWebServer supports this, but we can hack it in). >> > > > >>> >> > > > >>> The problem of port conflicts is annoying though. Maybe we try >> > random >> > > > >>> ports >> > > > >>> until one works? Is there any need to use the same port for >> > multiple >> > > > >>> runs? >> > > > >>> >> > > > >>> The CORS thing is sad, because it also means things like Camera >> > > plugin >> > > > >>> will >> > > > >>> be broken (can't use resulting URL in <img>). >> > > > >>> >> > > > >>> Although we might just do (this is different than the current >> > mapping >> > > > in >> > > > >>> the plugin): >> > > > >>> http://localhost:RANDOM_PORT/www >> > > > >>> http://localhost:RANDOM_PORT/asset-library >> > > > >>> http://localhost:RANDOM_PORT/file-system >> > > > >>> >> > > > >>> to proxy the three locations. >> > > > >>> >> > > > >>> This also means we can't use FileEntry.toURL() and have it work, >> > > unless >> > > > >>> toURL returns http://localhost:RANDOM_PORT/file-system/... >> Maybe >> > > > >>> that's >> > > > >>> fine? >> > > > >>> >> > > > >>> >> > > > >>> I don't think it's weird that an app will need to support >> WKWebView >> > > on >> > > > >>> some >> > > > >>> OS versions, and UIWebView on others. That's already the case to >> > > > support >> > > > >>> iOS 7. >> > > > >>> >> > > > >>> >> > > > >>> >> > > > >>> On Wed, Oct 29, 2014 at 6:22 PM, Shazron <shaz...@gmail.com> >> > wrote: >> > > > >>> >> > > > >>> > This does not preclude using the file url API function I >> suppose. >> > > > >>> Here's a >> > > > >>> > flowchart on how I think it would work: >> > > > http://i.imgur.com/zq4zreN.png >> > > > >>> > >> > > > >>> > >> > > > >>> > On Wed, Oct 29, 2014 at 2:48 PM, Tommy Williams < >> > > to...@devgeeks.org> >> > > > >>> > wrote: >> > > > >>> > >> > > > >>> > > This whole thing reeks of sadness and pain. >> > > > >>> > > >> > > > >>> > > The security implications of this will have to be considered >> > very >> > > > >>> > > carefully. >> > > > >>> > > On 29 Oct 2014 16:40, "Shazron" <shaz...@apache.org> wrote: >> > > > >>> > > >> > > > >>> > > > ## What We Know So Far >> > > > >>> > > > >> > > > >>> > > > 1. Because of the file:// url loading bug, we couldn't >> > support >> > > > the >> > > > >>> > > > WKWebView in the iOS 8 GM release. It has since been >> fixed, >> > for >> > > > >>> release >> > > > >>> > > > post iOS 8.1 (not sure when), through a new WKWebView API >> > > > function >> > > > >>> ( >> > > > >>> > > > http://trac.webkit.org/changeset/174029/trunk) >> > > > >>> > > > 2. The alternative is embedding a local web server and >> > serving >> > > > >>> assets >> > > > >>> > > from >> > > > >>> > > > that >> > > > >>> > > > >> > > > >>> > > > ## Abandon file:// url Loading API Method >> > > > >>> > > > >> > > > >>> > > > My proposal is, we abandon the local file:// url loading >> > method >> > > > in >> > > > >>> (1) >> > > > >>> > > > above, since it will create problems with support. >> > > > >>> > > > >> > > > >>> > > > For example, if we support the new local file url loading >> API >> > > > >>> function >> > > > >>> > in >> > > > >>> > > > iOS 8.2.0 (speculated) -- and a user is on 8.0.2, what >> then? >> > > You >> > > > >>> would >> > > > >>> > > not >> > > > >>> > > > have WKWebView support for that user, and you would use >> > > UIWebView >> > > > >>> > > instead. >> > > > >>> > > > This will just be confusing, and leads to problems. >> > > > >>> > > > >> > > > >>> > > > With the embedded local web server method, you can support >> > > > **any** >> > > > >>> > > version >> > > > >>> > > > of iOS 8.x. >> > > > >>> > > > >> > > > >>> > > > ## Embrace Embedded Local Web Server Method >> > > > >>> > > > >> > > > >>> > > > I have a Cordova plugin that implements this, and it >> should >> > > work >> > > > >>> with >> > > > >>> > > > cordova-ios 3.7.0: >> > > > >>> > > > https://github.com/shazron/CordovaLocalWebServer >> > > > >>> > > > >> > > > >>> > > > It dynamically updates the <access> tag value when it >> loads, >> > > > >>> overriding >> > > > >>> > > it >> > > > >>> > > > to the actual location and port. Since it is a plugin, it >> can >> > > be >> > > > >>> > > swappable >> > > > >>> > > > (for whatever reason). >> > > > >>> > > > >> > > > >>> > > > It does not solve the problem where any backgrounded app >> can >> > > > >>> access our >> > > > >>> > > > local web server. >> > > > >>> > > > >> > > > >>> > > > ## Future Steps >> > > > >>> > > > >> > > > >>> > > > This plugin is already working in cordova-ios 3.7.0 >> > > (un-released, >> > > > >>> up >> > > > >>> > next >> > > > >>> > > > for vote release). >> > > > >>> > > > >> > > > >>> > > > The wkwebview branch: >> > > > >>> > > > >> > > > >>> > > > 1. Needs be rebased >> > > > >>> > > > 2. Needs to be re-tested >> > > > >>> > > > 3. issues in CB-7043 that relate to the wkwebview have to >> be >> > > > >>> resolved >> > > > >>> > > > 4. branch presented for review to other committers >> > > > >>> > > > 5. resolve any comments and issues from (4) >> > > > >>> > > > 6. wkwebview branch integrated into master >> > > > >>> > > > >> > > > >>> > > > I will work on these items next after getting cordova-ios >> > 3.7.0 >> > > > >>> out. >> > > > >>> > Any >> > > > >>> > > > help is welcome. >> > > > >>> > > > >> > > > >>> > > > ## Migration Issues >> > > > >>> > > > >> > > > >>> > > > If you are migrating to WKWebView from UIWebView, you will >> > lose >> > > > >>> some >> > > > >>> > > > functionality. >> > > > >>> > > > >> > > > >>> > > > 1. No more whitelist feature (NSURLProtocol is not >> supported >> > in >> > > > >>> > > WKWebView) >> > > > >>> > > > 2. Your XmlHttpRequest calls now need to be CORS compliant >> > > (this >> > > > is >> > > > >>> > still >> > > > >>> > > > true if loaded through a file:// url) >> > > > >>> > > > 3. HTML5 offline application cache is not available in >> > > WKWebView >> > > > ( >> > > > >>> > > > https://devforums.apple.com/message/1060452) >> > > > >>> > > > >> > > > >>> > > >> > > > >>> > >> > > > >>> >> > > > >> >> > > > >> >> > > > > >> > > > >> > > >> > >> > >