cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian Clelland <iclell...@chromium.org>
Subject Re: [Android] Not so fast....API changes in Cordova-Android for Pluggable WebView
Date Thu, 01 May 2014 13:00:48 GMT
On Wed, Apr 30, 2014 at 5:44 PM, Andrew Grieve <agrieve@chromium.org> wrote:

> On Wed, Apr 30, 2014 at 5:35 PM, Joe Bowser <bowserj@gmail.com> wrote:
>
> > This is a perfect example of this XKCD: https://xkcd.com/1172/
> >
> > On Wed, Apr 30, 2014 at 2:25 PM, Ian Clelland <iclelland@chromium.org>
> > wrote:
> > > On Wed, Apr 30, 2014 at 3:58 PM, Andrew Grieve <agrieve@chromium.org>
> > wrote:
> > >
> > >> Config.xml is not a very sane way to do things for the embedded
> webview
> > >> case. E.g. you may want two webviews with different configs.
> > Config.java is
> > >> a singleton right now, and I think it would be much nicer as a
> parameter
> > >> you could give to the WebView upon initialization & plugins should
> say:
> > >> this.getConfig().getPreference() rather than using it as a singleton.
> > So...
> > >> If we could leave setPreference() in for now, I think we should. When
> we
> > >> remove it, we should provide a nice API for the embedding case (e.g. a
> > >> Config without the need to hit the filesystem).
> >
>
> Although I still think it shouldn't be a singleton, I've realized that
> plugins require it reading from config.xml, so an all-in-memory config
> doesn't make sense (would break plugins). Maybe for prefs it still would...
> not sure. Just thinking out loud on this one. Don't have a proposal.
>
>
>
> > >>
> > >>
> > >> pluginManager being public is unfortunate. That said, other than
> > >> getPlugin(), I don't see any methods in it that plugins should need.
> If
> > >> we're to remove the property, I don't think we should expose
> > PluginManager
> > >> to plugins, but rather try and keep that an internal detail.
> > >>
> > >
> > > This is actually what I was working on last night -- the problem is
> that
> > > plugins *do* use that field right now. File, File-Transfer, and Media
> > > Capture all use it to get access to other plugins to use their APIs.
> > > (through this.webView.pluginManager.getPlugin("somePlugin") )
> > >
> > > It does make sense for plugins to have native APIs as well as
> JavaScript
> > > APIs, and the only way to expose that right now is through the plugin
> > > manager.
> > >
> > > Unfortunately, it's been a public field on the plugin's webView object,
> > and
> > > there's no easy way to transition that to a setter. At least, not in a
> > way
> > > that ensures that both existing and new plugins can work with pre- and
> > > post-3.5.0 Cordova.
> >
>
> I think we're saying the same thing. I called out getPlugin() as the one
> plugins would actually need. It's things like init() shouldn't be touched
> by plugins.
>

Yep -- we appear to be in violent agreement with each other. I just missed
your original "other than getPlugin" disclaimer :)


>
> I don't see a clean way to not break this. I doubt it's used much by
> non-core plugins, so I think it's worth changing & doing a major semver
> bump for Android.
>

This seems like it should be a last resort -- it probably means that there
would be no more releases of those plugins for Android engines < 3.5.0(4?),
without a lot of work on our part. It would probably involve maintaining
two streams of releases for some time.


>
>
>
> > >
> > >
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >> On Wed, Apr 30, 2014 at 1:58 PM, Michal Mocny <mmocny@chromium.org>
> > wrote:
> > >>
> > >> > On Wed, Apr 30, 2014 at 1:30 PM, Joe Bowser <bowserj@gmail.com>
> > wrote:
> > >> >
> > >> > > On Wed, Apr 30, 2014 at 9:35 AM, Michal Mocny <
> mmocny@chromium.org>
> > >> > wrote:
> > >> > > > On Wed, Apr 30, 2014 at 12:23 PM, Joe Bowser <bowserj@gmail.com
> >
> > >> > wrote:
> > >> > > >
> > >> > > >> Hey
> > >> > > >>
> > >> > > >> So, once again, we're dealing with some major API changes
once
> we
> > >> > > >> introduce pluggable webview.  The first change that
was done
> for
> > >> > > >> sanity was finally deprecating setProperty.  This was
slated to
> > be
> > >> > > >> removed by 3.5 or in six months from the deprecation
date, but
> we
> > >> kept
> > >> > > >> it in too long.   While I would like to assume that
everyone
> has
> > >> moved
> > >> > > >> over to setting their preferences in config.xml, which
is the
> > much
> > >> > > >> more sane way of doing things, we can't do that.  We
need to
> > >> publicize
> > >> > > >> this in some blog posts, as well as in our documentation
> somehow.
> > >> > > >> There will obviously be some pissed off users, as we've
seen in
> > past
> > >> > > >> posts, but I think having the ability to use a WebView
other
> than
> > >> > > >> Chrome 30 is worth these changes.
> > >> > > >>
> > >> > > >
> > >> > > > Is it feasible to leave setProperty working only for default
> > WebView?
> > >> > >  This
> > >> > > > would mean that custom webviews won't work with older plugins,
> > but I
> > >> > > think
> > >> > > > thats fine.
> > >> > > >
> > >> > >
> > >> > > The setProperty methods are actually in Cordova-Activity, and
we
> > could
> > >> > > re-add those.  The thing is that these aren't actually used by
> > >> > > plugins, and instead are legacy methods that only our unit tests
> > use.
> > >> > > I'll put them back in.
> > >> > >
> > >> > > >
> > >> > > >>
> > >> > > >> The other change, which says more about our design is
adding a
> > >> getter
> > >> > > >> method for pluginManager.  We need to access the pluginManager
> to
> > >> get
> > >> > > >> plugins, and it's expected that everyone who implements
a
> > >> > > >> CordovaWebView will have this method produce a pluginManager.
>  In
> > >> the
> > >> > > >> past, it was just publicly exposed, which was not the
greatest
> > idea
> > >> > > >> and was kinda sloppy.
> > >> > > >>
> > >> > > >
> > >> > > > Similar to above question, could we leave it (deprecated)
as an
> > >> exposed
> > >> > > > property only on the default webview?  And only support
the new
> > >> getter
> > >> > > for
> > >> > > > new webviews (xwalk, gecko)?  Again, only updated plugins
would
> > work
> > >> > with
> > >> > > > custom webview, but I think thats fine.
> > >> > > >
> > >> > >
> > >> > > No, I don't think so.  It's probably better to make a clean break
> > and
> > >> > > have all the WebViews expected to function the same than to have
> > some
> > >> > > plugins simply fail with certain webviews.  Plugins breaking
> across
> > >> > > all the WebViews will force people to fix them, while things
> > breaking
> > >> > > with only Crosswalk will put crosswalk at an unfair disadvantage.
> > >> > >
> > >> >
> > >> > Trust me, Crosswalk is going to have an unfair *advantage*
> regardless
> > of
> > >> > plugin support ;)
> > >> >
> > >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message