1093209 - Handle "go back" session history menu Fringe cases

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivek Balakrishnan[:vivek]]

This is a followup to bug 847435 to handle fringe case such as :
1) Load few pages. Click on url bar. Long press back button.
Actual : History menu pops up
Expected : go back history should not popup while toolbar is in editing mode

2) Load few pages. Long press back button to pop up history. Minimize fennec app. Launch url from external app

Exected : Url should load in new tab and history popup should be dismissed
Actual : History popup is visible even when the url is loaded in new tab.

Comment 1

[Vivek Balakrishnan[:vivek]]

Attachment: Tab HIstory pop up Fringe cases

Tab History pop up is dismissed when 
* ulr toolbar is in editing mode
* url loaded from an external app.

Comment 2

[Brian Nicholson (:bnicholson)]

Comment on attachment 8519547 [details] [diff] [review]
Tab HIstory pop up Fringe cases

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

::: mobile/android/base/BrowserApp.java
@@ +257,5 @@
>      private final DynamicToolbar mDynamicToolbar = new DynamicToolbar();
>  
>      private TilesRecorder mTilesRecorder;
>  
> +    private boolean dimissTabHistoryonResume;

We should try to avoid using unnecessary class booleans if at all possible.

@@ +3078,5 @@
>              // Dismiss editing mode if the user is loading a URL from an external app.
>              mBrowserToolbar.cancelEdit();
>  
> +            // Mark tab history popup to be dismissed when loading a URL from an external app.
> +            dimissTabHistoryonResume = true;

Why do you need to set a boolean here instead of simply calling dismissTabHistoryFragment() directly?

Comment 3

[Vivek Balakrishnan[:vivek]]

Attachment: 1093209.patch

As per discussion over irc, tab history popup is dismissed when the activity goes to background.

Comment 4

[Brian Nicholson (:bnicholson)]

Comment on attachment 8520817 [details] [diff] [review]
1093209.patch

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

Nice.

Comment 5

[Brian Nicholson (:bnicholson)]

Don't forget, there are a couple of things that need to be done before we can set checkin-needed:
* The patch should contain the bug #, a summary of what was fixed, and the r=<reviewer> note.
* There should be a link to a try push in the bug.

Comment 6

[Vivek Balakrishnan[:vivek]]

Attachment: 1093209.patch

patch summary updated

Try run :
https://tbpl.mozilla.org/?tree=Try&rev=674bfe4f2881

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/648b2eb181ae

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/648b2eb181ae