cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: [Android] Not so fast....API changes in Cordova-Android for Pluggable WebView
Date Wed, 30 Apr 2014 21:44:01 GMT
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.

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.



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