856267 - Defect - Context Menu overlapping with the Tab App Bar when visible

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Input, defect, P1)

Description

[Kamil Jozwiak [:kjozwiak]]

If a context menu is visible at the top, it will overlap with the tab app bar.

Steps to reproduce the issue:

1) Open Firefox Metro
2) Go to mozilla.org
3) Tap and hold on the "Mission" link at the top of the page (a context menu should appear)
4) Once the context menu is visible near the top of the page, slide in the tab app bar

Current Behavior:

- The context menu will be displayed at the top of the tab app bar. Context Menu overlaps the tab app bar

Expected Behavior:

- Either the context menu is dismissed when a user slides in the tab app bar or its moved down with the web page. I think dismissing it is probably the better route

Comment 1

[Kamil Jozwiak [:kjozwiak]]

consideration for iteration #6

Comment 2

[Marco Mucci [:MarcoM]]

Moving to Iteration #6 for consideration as a Defect Story.

Comment 3

[Jim Mathies [:jimm]]

MenuUi needs to hide any menus when the app bar is displayed.

Comment 4

[Marco Mucci [:MarcoM]]

Jim's point estimate=1

Comment 5

[Brian R. Bondy [:bbondy]]

p=1

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: Hide menu popups when the appbar shows with a listener for MozContentShowing

Is there more to this than this listener? ContextMenu does a bit of its own housekeeping, so I can't have the popup itself listen and hide itself. 

The ContextMenuUI and AutofillMenuUI objects dont have any constructor or initializer to add the listener in, so I've simply dropped it at the end.

Comment 7

[Jim Mathies [:jimm]]

Could MenuPopup.prototype register for this in its show/hide methods and call the menu object back via a 'this' ptr passed to the MenuPopup(aPanel, aPopup) constructor?

Comment 8

[Sam Foster [:sfoster] (he/him)]

The MenuPopup instance has no references back to the thing that created it: 

  get _menuPopup() {
    if (!this.__menuPopup)
      this.__menuPopup = new MenuPopup(this._panel, this._popup);

    return this.__menuPopup;
  },

.. the constructor just receives the 2 elements that this._panel and this._popup resolve to, and inside MenuPopup's constructor, 'this' is the MenuPopup instance. So, no. We could pass a ref in (or assign one, like .control / .controller we use elsewhere) but would need to make sure this isn't by design, deliberately de-coupling the MenuPopup from whatever made it.

Comment 9

[Sam Foster [:sfoster] (he/him)]

Re-reading, I think that's what you meant - adding a pointer to the args passed to the MenuPopup constructor.

Comment 10

[Sam Foster [:sfoster] (he/him)]

Attachment: Add listener for MozContentShowing in MenuPopup so it hides itself (via its controller.hide method where available)

Elsewhere we use a thing.controller = this convention to assign a reference to the object that created it. I'd done the same here, which lets us add the MozAppbarShowing listener to the MenuPopup itself.

Comment 11

[Sam Foster [:sfoster] (he/him)]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Marco Mucci [:MarcoM]]

For testing and verification.

Comment 14

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.2; rv:23.0) Gecko/20130507 Firefox/23.0

This works for PC Immersive mode, but it would be good to double check on a tablet (don't have any here). Kamil, could you verify this?

Comment 15

[Kamil Jozwiak [:kjozwiak]]

Virgil, I don't have a tablet but I tried it on the touch screen laptop that was originally used to find the issue. Everything works without any problem, used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-08-03-11-13-mozilla-central/

- ensured that sliding in the "Tab App Bar" from the bottom dismissed the context menu
- ensured that sliding in the "Tab App Bar" from the top dismissed the context menu
- ensured that pressing "Windows + Z" dismisses the context menu
- ensured that creating a new tab from the side of the screen dismisses the context menu when returning to the original tab
- ensured that selecting the "Back" button on the side of the screen dismisses the context menu
- ensured that the context menu is dismissed when tapping somewhere else on the website
- still an issue when selecting "Help (online)" from the App Bar while the context menu (Bug 861536 was created for that issue)

I am marking this as verified, please let me know if the above is not enough and if this needs to be tested against real tablet and not a touch screen laptop.

Comment 16

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130819030205
Built from http://hg.mozilla.org/mozilla-central/rev/c8c9bd74cc40

WFM
Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment0 and got expected result.

Comment 17

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #16 without issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-10-16-03-02-02-mozilla-central/

- Went through the original issue from comment #0 without any issues
- Went through the test case(s) added in comment # 15 without any issues
- Ensured that the context menu is being cleared when going through the Windows Charm -> Settings (Options, Sync, About, Help (online) & Permissions)
- Ensured that all of the above test cases worked with both full & filled views

Issues:

- Bug 861536 is still occurring as of iteration #16
- Context menu not being cleared when accessing "Permissions" from "Settings".. Not consistent with the behavior the other times use (clearing context)