It is the right place to do it, otherwise we don't have the guarantee of
it invalidating ancestor sizes or anything.
It's also what we invalidate in WaylandPopupPropagateChangesToLayout().
We otherwise do not have the guarantee of SetPopupPosition running
before or after layout, if sizes do not change. That caused the popup
size to remain big, which caused a resize loop.
Differential Revision: https://phabricator.services.mozilla.com/D144314
Use mMoveToRectPopupRect popup constrains in nsMenuPopupFrame::SetPopupPosition only and make it pernament.
In such setup works as screen size constraints.
Don't call SetPopupPosition from move-to-rect callback and let layout code to handle that.
Remove MoveToRectPopupRectClear() as it's not needed, we keep mMoveToRectPopupRect
in nsMenuPopupFrame as well as we keep screen sizes there.
Differential Revision: https://phabricator.services.mozilla.com/D141602
Use mMoveToRectPopupRect popup constrains in nsMenuPopupFrame::SetPopupPosition only and make it pernament.
In such setup works as screen size constraints.
Don't call SetPopupPosition from move-to-rect callback and let layout code to handle that.
Remove MoveToRectPopupRectClear() as it's not needed, we keep mMoveToRectPopupRect
in nsMenuPopupFrame as well as we keep screen sizes there.
Differential Revision: https://phabricator.services.mozilla.com/D141602
Rename PreferredPopupRect* methods/attributes to MoveToRectPopupRect* ones as it holds info about coordinates received from move-to-rect popup placement done by Wayland compositor.
Differential Revision: https://phabricator.services.mozilla.com/D141425
What caused the issue was that nsMenuPopupFrame::MoveTo didn't account
for the change in bug 312891. But our context menu / popup positioning
code can be much simpler if we account for the context menu offset
before-hand as an extra horizontal / vertical margin in all directions.
Then we don't need to special-case it at all.
Differential Revision: https://phabricator.services.mozilla.com/D140263
What caused the issue was that nsMenuPopupFrame::MoveTo didn't account
for the change in bug 312891. But our context menu / popup positioning
code can be much simpler if we account for the context menu offset
before-hand as an extra horizontal / vertical margin in all directions.
Then we don't need to special-case it at all.
Differential Revision: https://phabricator.services.mozilla.com/D140263
Store preferred rect in dev pixels, which simplifies a bit other calculations,
and ensures that the nsMenuPopupFrame code accounts for zoom etc.
Use existing conversion methods for GDK <-> Device <-> CSS conversions.
Differential Revision: https://phabricator.services.mozilla.com/D139623
I don't see why ever mScreenRect would deal with zoom-less coordinates.
I _suspect_ this might have been accounting for screenX/Y not dealing
with zoom in the past, but that was fixed (not long ago, actually) in
bug 1753836.
Also, browser.xhtml never has zoom. I haven't seen this causing problems
in practice in non-Wayland platforms, but I think it's the right thing
to do and the code as is is extremely confusing, since other places
applying screen rects and context menu offsets treat them as real CSS
pixels:
https://searchfox.org/mozilla-central/rev/73a8000b0c0eb527faef01ea17c6d2398622a386/layout/xul/nsMenuPopupFrame.cpp#2407-2411
So I don't think this changes behavior in practice in non-wayland
platforms given we never have full zoom in browser.xhtml and we mostly
hit this code path for context menus, but given the above I think it's
simpler and more correct.
Depends on D139670
Differential Revision: https://phabricator.services.mozilla.com/D139671
This is drive-by but I hit this assert consistently when running
browser/components/extensions/test/browser/browser_ext_popup_background.js
locally.
The assert was wrong when we're recreating the widget, it is expected that
we've already constructed frames for its contents.
Depends on D133771
Differential Revision: https://phabricator.services.mozilla.com/D133772
Bug 1696718 landed a fix to propagate the position change from layout.
However that's not correct, because the widget isn't resized until
nsView::ProcessPendingUpdatesForView is resized (and even that in some
platforms it might be async).
So the right place to propagate the position change is in
nsXULPopupManager (which we call into from the view system which listens
itself to the widget).
Let's try to enable the test for that bug everywhere with this fixed.
Differential Revision: https://phabricator.services.mozilla.com/D127801
Make sure it's always in sync with the style.
Keeping a boolean field was useful when we didn't handle dynamic changes
to it, but now we do it's just redundant, and could cause correctness
issues.
Differential Revision: https://phabricator.services.mozilla.com/D124081
We have to use wayland to position popup windows and we need to propagate the modified
position (if happens) back to nsView. The mScreenRect in the nsMenuPopupFrame is in the CSS units
and that produce rounding error when font scale factor is not 1. To fix that we will use appunits
for the mScreenRect.
Differential Revision: https://phabricator.services.mozilla.com/D114577
It's unused on mozilla-central, and Thunderbird can just use the canvas
frame as regular (X)HTML documents, so just use a canvas frame instead
of an nsRootBoxFrame for XUL as well.
nsRootBoxFrame was needed because of various XUL-specific things like
tooltips and so on lived there. But with the move away from XUL, that
functionality has been added to nsCanvasFrame already, behind a
principal check instead.
This also allows simplifying our background propagation setup, which was
only half-working for XUL documents (this bug is a consequence of that).
With this, most of the callers of nsCSSRendering::IsCanvasFrame can go.
They're only two of the frames that would return true for that that
actually paint backgrounds (nsCanvasFrame and nsRootBoxFrame), so the
codepaths in display list building and painting can just check
frame->IsCanvasFrame() instead.
The remaining caller to that function is
nsContainerFrame::SyncWindowProperties, and the change is also legit, in
the sense that the only thing SyncWindowProperties() really cares about
is propagating the max/min-width constraints from the root element's
style to the view/widget, and the only frame that would return true from
IsCanvasFrame and have a view is the viewport frame which is the root of
the frame tree.
Differential Revision: https://phabricator.services.mozilla.com/D90846
- Clear mAdjustOffsetForContextMenu at nsMenuPopupFrame when running on Wayland and use move-to-rect to produce the offset.
- Implement nsWindow::WaylandPopupIsContextMenu()
- Use mBonuds directly in NativeMoveResizeWaylandPopupCallback() instead of Gtk query.
- Add some more loggin and code polishing.
Differential Revision: https://phabricator.services.mozilla.com/D117283
- Clear mAdjustOffsetForContextMenu at nsMenuPopupFrame when running on Wayland and use move-to-rect to produce the offset.
- Implement nsWindow::WaylandPopupIsContextMenu()
- Use mBonuds directly in NativeMoveResizeWaylandPopupCallback() instead of Gtk query.
- Add some more loggin and code polishing.
Differential Revision: https://phabricator.services.mozilla.com/D117283
So that margin is not included in the rect for visibility calculations,
and padding and margin are accounted properly on them.
Differential Revision: https://phabricator.services.mozilla.com/D113853
Now that nsMenuPopupFrame considers itself open while the native menu is shown,
it may try to open its non-native widget if layout happens to run while the
native menu is shown.
This check prevents that.
Differential Revision: https://phabricator.services.mozilla.com/D111326
This fixes tests which query popupElement.state.
Only the "root" menupopup is updated; the state for submenus is still always "closed".
Depends on D110973
Differential Revision: https://phabricator.services.mozilla.com/D110974
This is what non-native menus do. document.popupNode is a very awkward API; it's
global state that gets updated from multiple places and which is hard to reason
about.
For that reason, I think it's safest to stick closely to what non-native menus
are doing. I'm not aware of any user-facing breakage that this patch fixes.
Differential Revision: https://phabricator.services.mozilla.com/D110303
This is more in line with how non-native popups are managed. The popup manager
knows about everything and is the source of any state changes, and it updates
the nsMenuPopupFrame when necessary.
Concretely, this change makes these two upcoming changes easier:
- "Rolling up" native context menus. The popup manager is the nsIRollupListener.
- Returning something sensible from nsXULPopupManager::GetLastTriggerNode while
a native context menu is open.
Differential Revision: https://phabricator.services.mozilla.com/D110300
This is copied from the nsMenuPopupFrame::ShowPopup just a few lines below.
The native menu consumes mouseup events so we need to clear :active and capturing content just before opening the menu.
Differential Revision: https://phabricator.services.mozilla.com/D109672
This is macOS only and behind the prefs widget.macos.native-context-menus and
browser.proton.contextmenus.enabled .
The big design question here is: Where do we put the fork in the road? How much
should the existing non-native popup management machinery know about the state
of the native menu? Which parts of the non-native state should a) reflect the
true native state, b) enter a special "handled natively" state, or c) lie?
This patch picks the following approach:
- The nsMenuPopupFrame of the root menupopup knows about the native menu; it
keeps it alive in its new mNativeMenu field.
- If the context menu has submenus, i.e. nested <menupopup> elements, the
nsMenuPopupFrames for those nested menupopups do not know anything about the
native menu.
- The mPopupState of natively-handled nsMenuPopupFrames is ePopupClosed.
- XULPopupElement::GetState, which queries the frame's mPopupState, also
returns "closed". This might cause problems in the future.
- The XUL popup manager's mPopups "menu chain" does not know about any open
native menus.
- The rollup widget also does not know about the native popup.
I've chosen to use ePopupClosed for native menus for the following reasons:
1. While mirroring / updating the state for the root menu would be doable
without too much trouble, it would be much more annoying to do the same for
nested menupopups of submenus. So if submenus will be ePopupClosed, it's
consistent for the root menu to also be ePopupClosed.
2. nsXULPopupManager has assertions (for example in MayShowPopup) that make
sure that the menu popup frame's mPopupState is consistent with the XUL
popup manager's mPopups menu chain. Keeping the two both "closed" is the
easiest way to achieve consistency.
Unless there are grave concerns with this approach, I suggest we go with it for
now and see what trouble arises.
Differential Revision: https://phabricator.services.mozilla.com/D109183
Emilio Cobos Álvarez
stransky
Norisz Fay
Noemi Erli
Gijs Kruitbosch
Cristian Tuns
Hiroyuki Ikezoe
Kagami Sascha Rosylight
Butkovits Atila
Alexandru Michis
Markus Stange
Neil Deakin