1038793 - Minimize unused space in WebView of search activity

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[Eric Edens [:eedens]]

Two items here:
 (1) Move provider branding somewhere outside the webview
 (2) Move facet search buttons to native search buttons outside webview.

Comment 1

[Eric Edens [:eedens]]

Attachment: bug-1038793.patch

Punting on creating native facet buttons for another bug.

Comment 2

[:Margaret Leibovic]

Comment on attachment 8457500 [details] [diff] [review]
bug-1038793.patch

Review of attachment 8457500 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/search/java/org/mozilla/search/PostSearchFragment.java
@@ +22,5 @@
>      private static final String LOGTAG = "PostSearchFragment";
>      private WebView webview;
>  
> +    private static String HIDE_BANNER_SCRIPT = "javascript:(function(){var tag=document.createElement('style');" +
> +            "tag.type='text/css';document.getElementsByTagName('head')[0].appendChild(tag);tag.innerText='#nav,#header{display:none}'})();";

This is actually not as terrible as I thought it would be ;)

@@ +37,5 @@
>  
>          webview = (WebView) mainView.findViewById(R.id.webview);
> +        webview.setWebViewClient(new LinkInterceptingClient());
> +        webview.setWebChromeClient(new JsInjectingChrome());
> +        WebSettings webSettings = webview.getSettings();

Nit: declare this as final (or don't use the local variable at all).

@@ +87,5 @@
> +    /**
> +     * A custom WebChromeClient that allows us to inject CSS into
> +     * the head of the HTML.
> +     */
> +    private class JsInjectingChrome extends WebChromeClient {

Nit: I think a better name for this would be StyleInjectingChromeClient (or just StyleInjectingClient).

@@ +90,5 @@
> +     */
> +    private class JsInjectingChrome extends WebChromeClient {
> +
> +        @Override
> +        public void onReceivedTitle(WebView view, String title) {

We discussed this IRL, but you should add a comment explaining why we're choosing to do this when we receive the title, as opposed to at some other time during the page load.

::: mobile/android/search/res/values/search_dimens.xml
@@ +6,5 @@
>      <!-- Default screen margins, per the Android Design guidelines. -->
>      <dimen name="activity_horizontal_margin">16dp</dimen>
>      <dimen name="activity_vertical_margin">16dp</dimen>
> +
> +    <dimen name="webview_height_offset">55dp</dimen>

Where does this value come from?

Comment 3

[Eric Edens [:eedens]]

Attachment: bug-1038793.v2.patch

Thanks Margaret! Can you push to fx-team please? I just pulled it into master of the GH repo.

Comment 4

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/4034177d4227

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4034177d4227