This is self-contained and should live in a separate view to unbloat BrowserToolbar a bit.

Comment on attachment 831600 [details] [diff] [review] Factor out editing UI into ToolbarEditLayout (r=sriram) More chopping coming soon.

Comment on attachment 831599 [details] [diff] [review] Remove unused imports from BrowserToolbar (r=sriram) Review of attachment 831599 [details] [diff] [review]: ----------------------------------------------------------------- import omg.that.is.better;

Comment on attachment 831600 [details] [diff] [review] Factor out editing UI into ToolbarEditLayout (r=sriram) Review of attachment 831600 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Please change the mContext to getContext(). ::: mobile/android/base/toolbar/ToolbarEditLayout.java @@ +26,5 @@ > +import android.widget.ImageButton; > + > +public class ToolbarEditLayout extends GeckoLinearLayout { > + > + private final Context mContext; This is not needed. Use getContext() when needed. @@ +105,5 @@ > + final String contentDescription; > + > + if (textType == TextType.SEARCH_QUERY) { > + imageResource = R.drawable.ic_url_bar_search; > + contentDescription = mContext.getString(R.string.search); getContext() is sufficient.

(In reply to Sriram Ramasubramanian [:sriram] from comment #5) > Comment on attachment 831600 [details] [diff] [review] > Factor out editing UI into ToolbarEditLayout (r=sriram) > > Review of attachment 831600 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good to me. Please change the mContext to getContext(). > > ::: mobile/android/base/toolbar/ToolbarEditLayout.java > @@ +26,5 @@ > > +import android.widget.ImageButton; > > + > > +public class ToolbarEditLayout extends GeckoLinearLayout { > > + > > + private final Context mContext; > > This is not needed. Use getContext() when needed. > > @@ +105,5 @@ > > + final String contentDescription; > > + > > + if (textType == TextType.SEARCH_QUERY) { > > + imageResource = R.drawable.ic_url_bar_search; > > + contentDescription = mContext.getString(R.string.search); > > getContext() is sufficient. Fair enough, done. I do wonder why you didn't catch that on the ToolbarEditText patch :-)

https://hg.mozilla.org/integration/fx-team/rev/eb8ef0e80c9c https://hg.mozilla.org/integration/fx-team/rev/ea5939461476

Backed out for suspicion of causing mass robocop failures. https://hg.mozilla.org/integration/fx-team/rev/fa367f4785cb https://tbpl.mozilla.org/php/getParsedLog.php?id=30965139&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=30963985&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=30964585&tree=Fx-Team

Turned out to be some sort of needs-clobber situation. Re-landed and confirmed green. https://hg.mozilla.org/integration/fx-team/rev/d37c31a42f31 https://hg.mozilla.org/integration/fx-team/rev/0b75c809f3f7

https://hg.mozilla.org/mozilla-central/rev/d37c31a42f31 https://hg.mozilla.org/mozilla-central/rev/0b75c809f3f7