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)
> > > >>> > > >
> > > >>> > >
> > > >>> >
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> >
>
