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 15:45:03 GMT
I think I can get something up by the end of the day here (should still be
the middle of your day :) )
I'll let you know when I have anything that you can evaluate.


On Thu, May 1, 2014 at 11:39 AM, Joe Bowser <bowserj@gmail.com> wrote:

> Sounds like a pretty serious change, but I think we should do it.
> That being said, we're going to have to toss out all our existing
> JUnit tests.  How soon can you get the changes into the branch?
>
> On Thu, May 1, 2014 at 8:02 AM, Ian Clelland <iclelland@chromium.org>
> wrote:
> > The only way I can see out of this mess is to *not* turn CordovaWebView
> > into an interface, but instead make it an abstract base class. (I'm not
> > sure yet whether it has to inherit from anything; maybe Joe's original
> > refactorings mean that it can inherit only from Object, but I'm not sure
> > that it doesn't need to be a View)
> >
> > What I would do, then, is create a concrete AndroidCordovaWebView class
> > which extends it, and which has an AndroidWebView as a member.
> > (AndroidWebView still extends android.webkit.WebView, like it does now).
> > This would parallel the XWalkCordovaWebView class, which contains a
> private
> > XWalkView member.
> >
> > I think that the job of AndroidCordovaWebView would be to manage
> > cordova-related things, like plugins, and to delegate all of the
> web-viewy
> > things to its member WebView. I'm trying to work out the exact dividing
> > line between them; I suspect that I require more coffee to figure that
> one
> > out.
> >
> > Joe -- does this seem like a workable solution? It's essentially already
> > the way that the crosswalk plugin works, and I think that it solves the
> > issue of pluginManager needing to remain a public member. There may be
> > other implications that I just haven't seen yet, though.
> >
> > Ian
> >
> >
> > On Thu, May 1, 2014 at 9:32 AM, Ian Clelland <iclelland@chromium.org>
> wrote:
> >
> >> 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