Closed Bug 1093209 Opened 6 years ago Closed 6 years ago

Handle "go back" session history menu Fringe cases

Categories

(Firefox for Android :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 36

People

(Reporter: vivek, Assigned: vivek, Mentored)

References

Details

(Whiteboard: [lang=java])

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → vivekb.balakrishnan
Whiteboard: [lang=java]
Depends on: 847435
Blocks: 847435
Status: NEW → ASSIGNED
No longer depends on: 847435
Attached patch Tab HIstory pop up Fringe cases (obsolete) — Splinter Review
Tab History pop up is dismissed when 
* ulr toolbar is in editing mode
* url loaded from an external app.
Attachment #8519547 - Flags: review?(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?
Attachment #8519547 - Flags: review?(bnicholson) → review-
Attached patch 1093209.patch (obsolete) — Splinter Review
As per discussion over irc, tab history popup is dismissed when the activity goes to background.
Attachment #8519547 - Attachment is obsolete: true
Attachment #8520817 - Flags: review?(bnicholson)
Comment on attachment 8520817 [details] [diff] [review]
1093209.patch

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

Nice.
Attachment #8520817 - Flags: review?(bnicholson) → review+
Keywords: checkin-needed
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.
Flags: needinfo?(vivekb.balakrishnan)
Keywords: checkin-needed
Attached file 1093209.patch
patch summary updated

Try run :
https://tbpl.mozilla.org/?tree=Try&rev=674bfe4f2881
Attachment #8520817 - Attachment is obsolete: true
Flags: needinfo?(vivekb.balakrishnan)
Attachment #8520972 - Flags: review?(bnicholson)
Attachment #8520972 - Flags: review?(bnicholson) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/648b2eb181ae
Keywords: checkin-needed
Whiteboard: [lang=java] → [lang=java][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/648b2eb181ae
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][fixed-in-fx-team] → [lang=java]
Target Milestone: --- → Firefox 36
Depends on: 1120441
You need to log in before you can comment on or make changes to this bug.