This would alleviate a lot of the nasty interactions with Fennec and I think provide a better overall experience.

WIP

WIP

This overhauls WebAppActivity to use GeckoView rather than extending from GeckoApp. Depends partly on layout changes made in the patch for bug 1356346, which will be landed simultaneously.

Comment on attachment 8871303 [details] [diff] [review] GeckoView-based web apps Review of attachment 8871303 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java @@ +100,5 @@ > > final View customView = actionBar.getCustomView(); > mUrlView = (TextView) customView.findViewById(R.id.webapps_action_bar_url); > > + setGeckoInterface(new BaseGeckoInterface(this)); `BaseGeckoInterface` is gone and you don't need to call `setGeckoInterface` here anymore.

Comment on attachment 8871303 [details] [diff] [review] GeckoView-based web apps Big apologies, due to my photon commitments I havent been able to review this as thoroughly as I would have liked, however I generally agree with with the move towards geckoview so I do not want to block its progress. - EventDispatcher.getInstance().registerUiThreadListener(this, - "Website:AppEntered", - "Website:AppLeft", - null); Can we not do this in the same manner using geckoview? I find the need for Fennec to reimplement logic we hold inside gecko problematic and would like to see us improve the code sharing and less duplicating bugs. + private boolean isInScope(String url) { + if (mScope == null) { + return true; + } + ..... + + if (scopeSegments.size() > urlSegments.size()) { + return false; + } This is wrong, if scope is not defined then the default scope is the path of the start_url, ie if http://example.org/app/index.html is installed then http://example.org/app/hello/path/ is within scope The scope issue definitely needs fixed, I will give this thorough testing and follow up with bugs + fixes when it lands, thanks for the work

Thanks for the review! One thing: (In reply to Dale Harvey (:daleharvey) from comment #5) > This is wrong, if scope is not defined then the default scope is the path of > the start_url, ie if http://example.org/app/index.html is installed then > http://example.org/app/hello/path/ is within scope I thought this was the case too, but according to the spec[0], the correct behavior in absence of specified scope is to consider everything in scope. It might be worth considering ignoring the spec here given how odd that behavior is, but at any rate it's an easy enough change to make if we decide to do so. Of course there are probably other cases where scope handling is broken here (relative scopes, if nothing else). [0] https://www.w3.org/TR/appmanifest/#dfn-navigation-scope -- "If scopeURL is undefined (i.e., it is unbounded because of an error or it was not declared in the manifest), return true."

Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3e9ceef9b280 Overhaul WebAppActivity to use GeckoView rather than GeckoApp. r=jchen,daleharvey

This got backed out and will re-land in 56.

Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a4e85d16a5e2 Overhaul WebAppActivity to use GeckoView rather than GeckoApp. r=jchen,daleharvey