On Wed, Apr 30, 2014 at 5:44 PM, Andrew Grieve <agri...@chromium.org> wrote:
> On Wed, Apr 30, 2014 at 5:35 PM, Joe Bowser <bows...@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 <iclell...@chromium.org> > > wrote: > > > On Wed, Apr 30, 2014 at 3:58 PM, Andrew Grieve <agri...@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 <mmo...@chromium.org> > > wrote: > > >> > > >> > On Wed, Apr 30, 2014 at 1:30 PM, Joe Bowser <bows...@gmail.com> > > wrote: > > >> > > > >> > > On Wed, Apr 30, 2014 at 9:35 AM, Michal Mocny < > mmo...@chromium.org> > > >> > wrote: > > >> > > > On Wed, Apr 30, 2014 at 12:23 PM, Joe Bowser <bows...@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 ;) > > >> > > > >> > > >
