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 <[email protected]> wrote: > On Thu, May 1, 2014 at 9:00 AM, Ian Clelland <[email protected]>wrote: > >> On Wed, Apr 30, 2014 at 5:44 PM, Andrew Grieve <[email protected]>wrote: >> >>> On Wed, Apr 30, 2014 at 5:35 PM, Joe Bowser <[email protected]> wrote: >>> >>> > This is a perfect example of this XKCD: https://xkcd.com/1172/ >>> > >>> > On Wed, Apr 30, 2014 at 2:25 PM, Ian Clelland <[email protected]> >>> > wrote: >>> > > On Wed, Apr 30, 2014 at 3:58 PM, Andrew Grieve <[email protected] >>> > >>> > 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 <[email protected]> >>> > wrote: >>> > >> >>> > >> > On Wed, Apr 30, 2014 at 1:30 PM, Joe Bowser <[email protected]> >>> > wrote: >>> > >> > >>> > >> > > On Wed, Apr 30, 2014 at 9:35 AM, Michal Mocny < >>> [email protected]> >>> > >> > wrote: >>> > >> > > > On Wed, Apr 30, 2014 at 12:23 PM, Joe Bowser < >>> [email protected]> >>> > >> > 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 ;) >>> > >> > >>> > >> >>> > >>> >> >> >
