905088 - Properly handle default URL in enterEditingMode

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Lucas Rocha (:lucasr)]

Passing null on the param-less version is not exactly correct. Default value handling should be handled there. null URL param is an invalid call and should throw.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Properly handle default URL on BrowserApp's enterEditingMode

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 790119 [details] [diff] [review]
Properly handle default URL on BrowserApp's enterEditingMode

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

LGTM.

::: mobile/android/base/BrowserApp.java
@@ +1367,5 @@
>  
>      private void enterEditingMode() {
> +        String url = null;
> +
> +        Tab tab = Tabs.getInstance().getSelectedTab();

nit: I'm not sure what our policy is on the "final" keyword but...

final Tab tab = ...

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #2)
> Comment on attachment 790119 [details] [diff] [review]
> Properly handle default URL on BrowserApp's enterEditingMode
> 
> Review of attachment 790119 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM.
> 
> ::: mobile/android/base/BrowserApp.java
> @@ +1367,5 @@
> >  
> >      private void enterEditingMode() {
> > +        String url = null;
> > +
> > +        Tab tab = Tabs.getInstance().getSelectedTab();
> 
> nit: I'm not sure what our policy is on the "final" keyword but...
> 
> final Tab tab = ...

Nice catch, done.

Comment 4

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/projects/fig/rev/3eb68df2a650

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3eb68df2a650