[wayland] CreateWidgetForPopup() needs widget from parent menu on Wayland

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P2
normal
RESOLVED FIXED
3 months ago
17 days ago

People

(Reporter: stransky, Assigned: stransky)

Tracking

(Blocks 2 bugs, Regressed 1 bug)

Trunk
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 wontfix, firefox67 wontfix, firefox68 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

3 months ago

Wayland protocol allows to have only one popup window attached to the parent widget.

Recently we use a toplevel window widget as parent for all popups created by nsMenuPopupFrame::CreateWidgetForView(). That means second level menus (like File -> New Container Tab) are not displayed as both ("File" and "New Container Tab" menus) have the same parent widget.

Assignee

Comment 1

3 months ago

Wayland protocol allows to have only one popup window attached to a parent widget.

Recently we use a toplevel window widget as parent for all popups created
by nsMenuPopupFrame::CreateWidgetForView(). That means a second level menu
(like File -> New Container Tab) is not displayed as both
("File" and "New Container Tab" menus) have the same parent widget.

As a solution pass near parent menu widget to CreateWidgetForPopup()
to allow toolkit to create correct popup hierarchy at tooklit level.

Marking as wontfix for 67 as Wayland support won't ship in that release.

Assignee

Comment 5

2 months ago

Neil, can you be please more concrete which menus/popups needs to be covered? I tried various popups/menus and everything works fine. I tried bookmarks library and all menus also work fine there (popups, menus...). Thanks.

Flags: needinfo?(enndeakin)
Assignee

Updated

2 months ago
Blocks: 1539471

Comment 6

2 months ago

There are lots of places where a context menu can be opened while another menu is open. For example, open a bookmark folder on the bookmarks toolbar, which will open as a menu, then open a context menu for a specific bookmark. Tooltips also are assigned for each bookmark item since labels can be cropped.

The context menu or tooltips, or the dropdowns of <select> elements can also be opened for any panel that opens from an extension.

Flags: needinfo?(enndeakin)
Assignee

Comment 7

2 months ago

(In reply to Neil Deakin (away march 22 - 31) from comment #6)

There are lots of places where a context menu can be opened while another
menu is open. For example, open a bookmark folder on the bookmarks toolbar,
which will open as a menu, then open a context menu for a specific bookmark.
Tooltips also are assigned for each bookmark item since labels can be
cropped.

The context menu or tooltips, or the dropdowns of <select> elements can also
be opened for any panel that opens from an extension.

Yes, I think I understand you now.

The content menus created/opened when a different menu is active is a different case and needs an extra approach. One content menu can have different parents as it's reused and that needs to be addressed when the popup is rendered and not when it's created.

Assignee

Comment 8

2 months ago

Wayland protocol allows to have only one popup window attached to a parent widget.

Recently we use a toplevel window widget as parent for all popups.
That means a second level menu (like File -> New Container Tab)
is not displayed as both ("File" and "New Container Tab" menus)
have the same parent widget.

As a solution allow to get the actual parent run-time and set that
when we open the window on toolkit level.

This patch covers menu widgets in the same frame hierarchy.

Assignee

Updated

2 months ago
Attachment #9048502 - Attachment is obsolete: true
Assignee

Comment 9

2 months ago

Fro the widgets in different frame hierarchy we need to use run-time popup tracking as we have in Bug 1539471, so I'll extend Bug 1539471 for it.

Assignee

Updated

2 months ago
Blocks: 1543600
Assignee

Comment 10

a month ago

Ping. Neil, any update about the review? It's blocking all further wayland popup fixes. Thanks.

Flags: needinfo?(enndeakin)

Updated

a month ago
Flags: needinfo?(enndeakin)
Assignee

Updated

a month ago
Keywords: checkin-needed

Comment 11

a month ago

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b9a9068783f7
[Wayland] Set popup window hierarchy run-time on Wayland for menus in the same frame hierarchy, r=NeilDeakin

Keywords: checkin-needed

Comment 12

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Regressions: 1548496
You need to log in before you can comment on or make changes to this bug.