Handle "go back" session history menu Fringe cases

RESOLVED FIXED in Firefox 36

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vivek, Assigned: vivek, Mentored)

Tracking

Trunk
Firefox 36
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=java])

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

5 years ago
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

Updated

5 years ago
Assignee: nobody → vivekb.balakrishnan
Whiteboard: [lang=java]
Assignee

Updated

5 years ago
Depends on: 847435
Blocks: 847435
Status: NEW → ASSIGNED
No longer depends on: 847435
Assignee

Comment 1

5 years ago
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-
Assignee

Comment 3

5 years ago
Posted 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+
Assignee

Updated

5 years ago
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
Assignee

Comment 6

5 years ago
Posted 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+
Assignee

Updated

5 years ago
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: 5 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.