Closed Bug 688840 Opened 8 years ago Closed 8 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

Firefox 9
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 9

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
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.
Attachment #562136 - Flags: review?(wjohnston)
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?
Attachment #562136 - Flags: review?(wjohnston) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/f3df57832f29
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 9
Duplicate of this bug: 688987
Duplicate of this bug: 688984
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
Status: RESOLVED → VERIFIED
Depends on: 690973
You need to log in before you can comment on or make changes to this bug.