Closed Bug 1184912 Opened 8 years ago Closed 8 years ago

Zoomed view stays open if I didn't close it and I switch apps


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox42 fixed)

Firefox 42
Tracking Status
firefox42 --- fixed


(Reporter: antlam, Assigned: domivinc)




(1 file, 2 obsolete files)


Trigger zoomed view
Leave it open
Switch apps
Open a link with Firefox
Zoomed view is still open from last time

I hope it's just me!
The zoomed view is stopped when the event "Content:LocationChange" is received.
It looks like we never received this event in your user case.
I'm going to fix this issue.
Assignee: nobody → domivinc
I tried to cover all the different cases: new tab, selection of another existing tab.
Attachment #8635743 - Flags: review?(michael.l.comella)
Comment on attachment 8635743 [details] [diff] [review]
patch-19072015 2-Bug_1184912___Close_zoomed_view_on_tab_change__r_mcomella.patch

Review of attachment 8635743 [details] [diff] [review]:

::: mobile/android/base/
@@ +2057,5 @@
>      }
>      @Override
>      public void addTab() {
> +        stopZoomDisplayIfVisible();

We should only need onTabChanged -> case SELECTED because when a tab is added to the foreground, it is selected, when a tab is added to the background (e.g. popup via page content script), it shouldn't be selected and we don't want to close the zoomed view, or when the current tab is removed, it is selected.

@@ +2063,5 @@
>      }
>      @Override
>      public void addPrivateTab() {
> +        stopZoomDisplayIfVisible();

As above, we shouldn't need this.

@@ +2289,5 @@
>          if (oldTab != null) {
>              oldTab.setIsEditing(false);
>          }
> +        stopZoomDisplayIfVisible();

This calls Tabs.selectTab, which calls notifyListeners on SELECTED so you should just be able to leave the reference in BrowserApp.onTabChanged.

::: mobile/android/base/
@@ +500,5 @@
>          moveUsingGeckoPosition(leftFromGecko, topFromGecko);
>      }
> +    public void stopZoomDisplayIfVisible() {

Why is this wrapper necessary? Can we just check if the view is visible in stopZoomDisplay directly? How does this interact with your work in bug 1184762 where we check for visibility in hideZoomedView?
Attachment #8635743 - Flags: review?(michael.l.comella) → review-
All changes done, patch of bug 1184762 updated.
Attachment #8635743 - Attachment is obsolete: true
Attachment #8637140 - Flags: review?(michael.l.comella)
Depends on: 1184762
Keywords: checkin-needed
Hi, this patch failed to apply:

renamed 1184912 -> patch-20072015_2-Bug_1184912___Close_zoomed_view_on_tab_change__r_mcomella.patch
applying patch-20072015_2-Bug_1184912___Close_zoomed_view_on_tab_change__r_mcomella.patch
patching file mobile/android/base/
Hunk #1 FAILED at 495
1 out of 1 hunks FAILED -- saving rejects to file mobile/android/base/
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh patch-20072015_2-Bug_1184912___Close_zoomed_view_on_tab_change__r_mcomella.patch

can you take a look, thanks!
Flags: needinfo?(domivinc)
Keywords: checkin-needed
Same patch re-based.
Flags: needinfo?(domivinc)
Keywords: checkin-needed
Attachment #8637140 - Attachment is obsolete: true
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.