Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cordova.apache.org
Received-SPF: pass (nike.apache.org: domain of shazron@gmail.com designates
 209.85.216.51 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAK_TSXJU2+oMcMcFCQ9Spp1F=H0d_buJwiH21uy1cUZXxffO_A@mail.gmail.com>
References: 
 <CAE6O_-aBHmyw6SKVHuuUZCxYaiaK4v2qh8401K2-SbtPtgm+7w@mail.gmail.com>
 <CAPTyS6DtkdNkkDGVe5bAwRTXUsBK7w_WHa4NDV4Rkrdf9uAQfA@mail.gmail.com>
 <CAE6O_-YsR9CN2GO8T6kkL13EzpQk4HhALV83wi4SKmz9qfFiMg@mail.gmail.com>
 <CABiQX1V7Y18-n=L97S+rwgNH_cMR0GWV4wm0JQ9FGs_1ty_E4g@mail.gmail.com>
 <CAE6O_-Zzg7RUsvavCuf_wUO8fC+1z09FukPT_ZH1ybXOueP4fw@mail.gmail.com>
 <CAE6O_-YzvFwL64Rt87P6Ow1CfbpSfJoZDuCzfcTPzRV14Bx58Q@mail.gmail.com>
 <CAE6O_-arBCRAswj_NOKN2VPivRmCP3MnByp2EmY1id=vF3PcfQ@mail.gmail.com>
 <CABiQX1XTCabXJFu9dwg5g3Ys+9zr=tLhsprfMSjQ927TQHe_Ew@mail.gmail.com>
 <CAKP6k2u+Ts7Z8YTASJg_H=KH24BUHW5gTGreOzi1ghO2A8E63A@mail.gmail.com>
 <CAK_TSXJU2+oMcMcFCQ9Spp1F=H0d_buJwiH21uy1cUZXxffO_A@mail.gmail.com>
From: Shazron <shazron@gmail.com>
Date: Wed, 19 Nov 2014 09:19:59 -0800
Message-ID: 
 <CAE6O_-bNbeVETr80_3y=-LZxmnj96mNrmqEBaG5frsJK2k8v7w@mail.gmail.com>
Subject: Re: [iOS 8] WKWebView moving forward
To: "dev@cordova.apache.org" <dev@cordova.apache.org>
Content-Type: multipart/alternative; boundary=001a11c2cf4648ff690508396ec4

--001a11c2cf4648ff690508396ec4
Content-Type: text/plain; charset=UTF-8

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 <iclelland@chromium.org>
wrote:

> On Tue Nov 18 2014 at 2:00:34 PM Jesse <purplecabbage@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 <agrieve@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 <shazron@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 <shazron@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 <shazron@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 <
> > agrieve@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 <shazron@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 <
> > > tommy@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" <shazron@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)
> > > > >>> > > >
> > > > >>> > >
> > > > >>> >
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > >
> > >
> >
>

--001a11c2cf4648ff690508396ec4--