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:32:39 GMT
On Thu, May 1, 2014 at 9:00 AM, Ian Clelland <iclelland@chromium.org> wrote:

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

Ultimately, I think you're right, though.

The issue isn't making a new release of the plugins that works with old and
new cordova engines. That's possible (though ugly) with reflection; I got
it working a couple of nights ago when the problem was first apparent.
The real issue is that there's no way for the existing published plugins to
possibly work with this change to the API. That's a
backwards-incompatibility no matter how you look at it, and means that we
need to bump the major.

Maybe we can use this as an opportunity to define a real inter-plugin
native API, as a feature of a Cordova 4 release?


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