688840 - [tabletui] Switching from portrait to landscape breaks tab sidebar toggling

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Steps to reproduce:
1. Open Fennec in tablet mode in portrait orientation
2. Tap the tabs button to open the popup tabs menu
3. Rotate to landscape orientation

Expected results: Portrait tabs menu disappears and landscape tabs sidebar is shown (unless you previously hid it)

Actual results: Portrait tabs menu is still present and landscape tabs sidebar is hidden.  The landscape sidebar can no longer be shown.

This patch fixes a few problems:
1. The removeEventListener call was failing because "this.resizeHandler" did not match the argument to addEventListener ("this.resizeHandler.bind(this)").
2. The resize handler was hiding the sidebar in response to resize events that were not targeted at the top-level window.
3. The resize handler called hide() which hid the landscape sidebar instead of the portrait menu.

Comment 1

[Wesley Johnston (:wesj)]

Comment on attachment 562136 [details] [diff] [review]
patch

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

::: mobile/chrome/content/TabsPopup.js
@@ +132,5 @@
> +  _hidePortraitMenu: function _hidePortraitMenu() {
> +    if (!this.box.hidden) {
> +      this.box.hidden = true;
> +      BrowserUI.popPopup(this);
> +      window.removeEventListener("resize", resizeHandler, false);

I don't think we want this every time this hides. Only when the event listener is called?

Comment 2

[Matt Brubeck (:mbrubeck)]

(In reply to Wesley Johnston (:wesj) from comment #1)
> > +      window.removeEventListener("resize", resizeHandler, false);
> 
> I don't think we want this every time this hides. Only when the event
> listener is called?

If we don't remove the listener on hide, then we'll end up with a duplicate listener added the next time show() is called.

Comment 3

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3df57832f29

Comment 4

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/f3df57832f29

Comment 7

[Cristian Nicolae (:xti)]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110926
Firefox/9.0a1 Fennec/9.0a1
Device: Acer ICONIA A500
OS: Android 3.1