Closed Bug 856267 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86_64
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 23

People

(Reporter: kjozwiak, Assigned: sfoster)

References

Details

(Whiteboard: feature=defect c=context_menus u=metro_firefox_user p=1)

Attachments

(1 file, 1 obsolete file)

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
consideration for iteration #6
Flags: needinfo?(mmucci)
Moving to Iteration #6 for consideration as a Defect Story.
Flags: needinfo?(mmucci)
Priority: -- → P1
QA Contact: jbecerra
Whiteboard: feature=defect c=context_menus u=metro_firefox_user p=0
Blocks: metrov1planning
No longer blocks: metrov1it6
Blocks: metrov1defect&change
No longer blocks: metrov1planning
MenuUi needs to hide any menus when the app bar is displayed.
Whiteboard: feature=defect c=context_menus u=metro_firefox_user p=0 → feature=defect c=context_menus u=metro_firefox_user p=1
Jim's point estimate=1
Whiteboard: feature=defect c=context_menus u=metro_firefox_user p=1 → feature=defect c=context_menus u=metro_firefox_user p=0
Priority: P1 → --
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.
Assignee: nobody → sfoster
Attachment #743113 - Flags: review?(jmathies)
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?
Blocks: metrov1it7
No longer blocks: metrov1defect&change
Whiteboard: feature=defect c=context_menus u=metro_firefox_user p=0 → feature=defect c=context_menus u=metro_firefox_user p=1
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.
Re-reading, I think that's what you meant - adding a pointer to the args passed to the MenuPopup constructor.
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.
Attachment #743113 - Attachment is obsolete: true
Attachment #743113 - Flags: review?(jmathies)
Attachment #743608 - Flags: review?(jmathies)
Attachment #743608 - Flags: review?(jmathies) → review+
Priority: -- → P1
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e562e90b820f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
For testing and verification.
Flags: needinfo?(mozbugs.retornam)
Flags: needinfo?(mozbugs.retornam) → needinfo?(virgil.dicu)
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?
Flags: needinfo?(virgil.dicu) → needinfo?(kamiljoz)
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.
Status: RESOLVED → VERIFIED
Flags: needinfo?(kamiljoz)
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.
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)
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.