Bug 1666497 - Don't flush layout from popuppositioned events. r=mconley,Gijs,smaug

We don't need to flush before dispatching the event because we know that
if the values we cared about changed, then we'd get another event.

Differential Revision: https://phabricator.services.mozilla.com/D92444
This commit is contained in:
Emilio Cobos Álvarez 2020-10-06 16:28:41 +00:00
parent 78ce42a419
commit 4ad501e86f
14 changed files with 118 additions and 109 deletions

View File

@ -23,15 +23,6 @@ const EXPECTED_APPMENU_OPEN_REFLOWS = [
],
},
{
stack: [
"adjustArrowPosition@chrome://global/content/elements/panel.js",
"on_popuppositioned@chrome://global/content/elements/panel.js",
],
maxCount: 22, // This number should only ever go down - never up.
},
{
stack: [
"_calculateMaxHeight@resource:///modules/PanelMultiView.jsm",
@ -71,7 +62,7 @@ add_task(async function() {
),
exceptions: [
{
name: "the urlbar placeolder moves up and down by a few pixels",
name: "the urlbar placeholder moves up and down by a few pixels",
condition: r =>
r.x1 >= textBoxRect.left &&
r.x2 <= textBoxRect.right &&

View File

@ -1147,7 +1147,7 @@ var PanelMultiView = class extends AssociatedToNode {
}
}
_calculateMaxHeight() {
_calculateMaxHeight(aEvent) {
// While opening the panel, we have to limit the maximum height of any
// view based on the space that will be available. We cannot just use
// window.screen.availTop and availHeight because these may return an
@ -1169,7 +1169,7 @@ var PanelMultiView = class extends AssociatedToNode {
// The distance from the anchor to the available margin of the screen is
// based on whether the panel will open towards the top or the bottom.
let maxHeight;
if (this._panel.alignmentPosition.startsWith("before_")) {
if (aEvent.alignmentPosition.startsWith("before_")) {
maxHeight = anchor.screenY - cssAvailTop;
} else {
let anchorScreenBottom = anchor.screenY + anchorRect.height;
@ -1229,7 +1229,7 @@ var PanelMultiView = class extends AssociatedToNode {
}
case "popuppositioned": {
if (this._panel.state == "showing") {
let maxHeight = this._calculateMaxHeight();
let maxHeight = this._calculateMaxHeight(aEvent);
this._viewStack.style.maxHeight = maxHeight + "px";
this._offscreenViewStack.style.maxHeight = maxHeight + "px";
}

View File

@ -573,6 +573,7 @@
const event_names = [
"popupshowing",
"popuppositioned",
"popupshown",
"transitionend",
"popuphiding",
@ -634,7 +635,7 @@
return this.shadowRoot.querySelector(".panel-arrow");
}
adjustArrowPosition() {
adjustArrowPosition(event) {
let arrow = this.arrow;
let anchor = this.anchorNode;
@ -646,8 +647,8 @@
let container = this.container;
let arrowbox = this.arrowbox;
var position = this.alignmentPosition;
var offset = this.alignmentOffset;
var position = event.alignmentPosition;
var offset = event.alignmentOffset;
this.setAttribute("arrowposition", position);
@ -700,11 +701,16 @@
on_popupshowing(event) {
if (event.target == this) {
this.adjustArrowPosition();
this.setAttribute("animate", "open");
}
}
on_popuppositioned(event) {
if (event.target == this) {
this.adjustArrowPosition(event);
}
}
on_popupshown(event) {
if (event.target != this) {
return;

View File

@ -415,8 +415,6 @@ NON_IDL_EVENT(command, eXULCommand, EventNameType_XUL, eInputEventClass)
NON_IDL_EVENT(popupshowing, eXULPopupShowing, EventNameType_XUL,
eBasicEventClass)
NON_IDL_EVENT(popupshown, eXULPopupShown, EventNameType_XUL, eBasicEventClass)
NON_IDL_EVENT(popuppositioned, eXULPopupPositioned, EventNameType_XUL,
eBasicEventClass)
NON_IDL_EVENT(popuphiding, eXULPopupHiding, EventNameType_XUL, eBasicEventClass)
NON_IDL_EVENT(popuphidden, eXULPopupHidden, EventNameType_XUL, eBasicEventClass)
NON_IDL_EVENT(broadcast, eXULBroadcast, EventNameType_XUL, eBasicEventClass)

View File

@ -0,0 +1,22 @@
/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/.
*/
dictionary PopupPositionedEventInit : EventInit {
/**
* Returns the alignment position where the popup has appeared relative to its
* anchor node or point, accounting for any flipping that occurred.
*/
DOMString alignmentPosition = "";
long alignmentOffset = 0;
};
[ChromeOnly, Exposed=Window]
interface PopupPositionedEvent : Event {
constructor(DOMString type, optional PopupPositionedEventInit init = {});
readonly attribute DOMString alignmentPosition;
readonly attribute long alignmentOffset;
};

View File

@ -180,11 +180,5 @@ interface XULPopupElement : XULElement
*/
void sizeTo(long width, long height);
/** Returns the alignment position where the popup has appeared relative to its
* anchor node or point, accounting for any flipping that occurred.
*/
readonly attribute DOMString alignmentPosition;
readonly attribute long alignmentOffset;
void setConstraintRect(DOMRectReadOnly rect);
};

View File

@ -1096,6 +1096,7 @@ GENERATED_EVENTS_WEBIDL_FILES = [
'PluginCrashedEvent.webidl',
'PopStateEvent.webidl',
'PopupBlockedEvent.webidl',
'PopupPositionedEvent.webidl',
'PositionStateEvent.webidl',
'PresentationConnectionAvailableEvent.webidl',
'PresentationConnectionCloseEvent.webidl',

View File

@ -230,69 +230,6 @@ already_AddRefed<DOMRect> XULPopupElement::GetOuterScreenRect() {
return rect.forget();
}
void XULPopupElement::GetAlignmentPosition(nsString& positionStr) {
positionStr.Truncate();
// This needs to flush layout.
nsMenuPopupFrame* menuPopupFrame =
do_QueryFrame(GetPrimaryFrame(FlushType::Layout));
if (!menuPopupFrame) return;
int8_t position = menuPopupFrame->GetAlignmentPosition();
switch (position) {
case POPUPPOSITION_AFTERSTART:
positionStr.AssignLiteral("after_start");
break;
case POPUPPOSITION_AFTEREND:
positionStr.AssignLiteral("after_end");
break;
case POPUPPOSITION_BEFORESTART:
positionStr.AssignLiteral("before_start");
break;
case POPUPPOSITION_BEFOREEND:
positionStr.AssignLiteral("before_end");
break;
case POPUPPOSITION_STARTBEFORE:
positionStr.AssignLiteral("start_before");
break;
case POPUPPOSITION_ENDBEFORE:
positionStr.AssignLiteral("end_before");
break;
case POPUPPOSITION_STARTAFTER:
positionStr.AssignLiteral("start_after");
break;
case POPUPPOSITION_ENDAFTER:
positionStr.AssignLiteral("end_after");
break;
case POPUPPOSITION_OVERLAP:
positionStr.AssignLiteral("overlap");
break;
case POPUPPOSITION_AFTERPOINTER:
positionStr.AssignLiteral("after_pointer");
break;
case POPUPPOSITION_SELECTION:
positionStr.AssignLiteral("selection");
break;
default:
// Leave as an empty string.
break;
}
}
int32_t XULPopupElement::AlignmentOffset() {
nsMenuPopupFrame* menuPopupFrame =
do_QueryFrame(GetPrimaryFrame(FlushType::Frames));
if (!menuPopupFrame) return 0;
int32_t pp = mozilla::AppUnitsPerCSSPixel();
// Note that the offset might be along either the X or Y axis, but for the
// sake of simplicity we use a point with only the X axis set so we can
// use ToNearestPixels().
nsPoint appOffset(menuPopupFrame->GetAlignmentOffset(), 0);
nsIntPoint popupOffset = appOffset.ToNearestPixels(pp);
return popupOffset.x;
}
void XULPopupElement::SetConstraintRect(dom::DOMRectReadOnly& aRect) {
nsMenuPopupFrame* menuPopupFrame =
do_QueryFrame(GetPrimaryFrame(FlushType::Frames));

View File

@ -85,10 +85,6 @@ class XULPopupElement : public nsXULElement {
void SizeTo(int32_t aWidth, int32_t aHeight);
void GetAlignmentPosition(nsString& positionStr);
int32_t AlignmentOffset();
void SetConstraintRect(DOMRectReadOnly& aRect);
protected:

View File

@ -39,6 +39,8 @@
#include "mozilla/dom/MouseEvent.h"
#include "mozilla/dom/UIEvent.h"
#include "mozilla/dom/UserActivation.h"
#include "mozilla/dom/PopupPositionedEvent.h"
#include "mozilla/dom/PopupPositionedEventBinding.h"
#include "mozilla/EventDispatcher.h"
#include "mozilla/LookAndFeel.h"
#include "mozilla/MouseEvents.h"
@ -2611,6 +2613,39 @@ bool nsXULPopupPositionedEvent::DispatchIfNeeded(nsIContent* aPopup) {
return false;
}
static void AlignmentPositionToString(nsMenuPopupFrame* aFrame,
nsAString& aString) {
aString.Truncate();
int8_t position = aFrame->GetAlignmentPosition();
switch (position) {
case POPUPPOSITION_AFTERSTART:
return aString.AssignLiteral("after_start");
case POPUPPOSITION_AFTEREND:
return aString.AssignLiteral("after_end");
case POPUPPOSITION_BEFORESTART:
return aString.AssignLiteral("before_start");
case POPUPPOSITION_BEFOREEND:
return aString.AssignLiteral("before_end");
case POPUPPOSITION_STARTBEFORE:
return aString.AssignLiteral("start_before");
case POPUPPOSITION_ENDBEFORE:
return aString.AssignLiteral("end_before");
case POPUPPOSITION_STARTAFTER:
return aString.AssignLiteral("start_after");
case POPUPPOSITION_ENDAFTER:
return aString.AssignLiteral("end_after");
case POPUPPOSITION_OVERLAP:
return aString.AssignLiteral("overlap");
case POPUPPOSITION_AFTERPOINTER:
return aString.AssignLiteral("after_pointer");
case POPUPPOSITION_SELECTION:
return aString.AssignLiteral("selection");
default:
// Leave as an empty string.
break;
}
}
NS_IMETHODIMP
nsXULPopupPositionedEvent::Run() {
nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
@ -2633,11 +2668,23 @@ nsXULPopupPositionedEvent::Run() {
if (state != ePopupPositioning && state != ePopupShown) {
return NS_OK;
}
nsEventStatus status = nsEventStatus_eIgnore;
WidgetMouseEvent event(true, eXULPopupPositioned, nullptr,
WidgetMouseEvent::eReal);
EventDispatcher::Dispatch(mPopup, popupFrame->PresContext(), &event, nullptr,
&status);
// Note that the offset might be along either the X or Y axis, but for the
// sake of simplicity we use a point with only the X axis set so we can
// use ToNearestPixels().
int32_t popupOffset = nsPoint(popupFrame->GetAlignmentOffset(), 0)
.ToNearestPixels(AppUnitsPerCSSPixel())
.x;
PopupPositionedEventInit init;
init.mComposed = true;
init.mAlignmentOffset = popupOffset;
AlignmentPositionToString(popupFrame, init.mAlignmentPosition);
RefPtr<PopupPositionedEvent> event =
PopupPositionedEvent::Constructor(mPopup, u"popuppositioned"_ns, init);
event->SetTrusted(true);
mPopup->DispatchEvent(*event);
// Get the popup frame and make sure it is still in the positioning
// state. If it isn't, someone may have tried to reshow or hide it

View File

@ -45,7 +45,7 @@ SimpleTest.waitForExplicitFinish();
const isOSXYosemite = navigator.userAgent.includes("Mac OS X 10.10");
var expectedAnchor = null;
var expectedSide = "", expectedAnchorEdge = "", expectedPack = "", expectedAlignment = "";
var expectedSide = "", expectedAnchorEdge = "", expectedPack = "";
var zoomFactor = 1;
var animatedPopupShown = false;
var animatedPopupHidden = false;
@ -67,7 +67,6 @@ function* nextTest()
expectedSide = expected;
expectedAnchorEdge = anchorEdge;
expectedPack = pack;
expectedAlignment = alignment == undefined ? position : alignment;
panel.removeAttribute("side");
panel.openPopup(expectedAnchor, position, 0, 0, false, false, null);
@ -298,7 +297,6 @@ function checkPanelPosition(panel)
is(panel.getAttribute("side"), expectedSide, "panel arrow side");
is(arrow.hidden, false, "panel hidden");
is(arrow.parentNode.getAttribute("pack"), expectedPack, "panel arrow pack");
is(panel.alignmentPosition, expectedAlignment, "panel alignmentPosition");
panel.hidePopup();
}

View File

@ -88,12 +88,31 @@ function openPopup(position, callback) {
_openPopup(position, callback);
}
function waitForPopupPositioned(actionFn, callback)
async function waitForPopupPositioned(actionFn, callback)
{
panel.addEventListener("popuppositioned", function listener() {
SimpleTest.executeSoon(callback);
}, {once: true});
info("waitForPopupPositioned");
let a = new Promise(resolve => {
panel.addEventListener("popuppositioned", () => resolve(true), { once: true });
});
actionFn();
// Ensure we get at least one event, but we might get more than one, so wait for them as needed.
//
// The double-raf ensures layout runs once. setTimeout is needed because that's how popuppositioned is scheduled.
let b = new Promise(resolve => {
requestAnimationFrame(() => requestAnimationFrame(() => {
setTimeout(() => resolve(false), 0);
}));
});
let gotEvent = await Promise.race([a, b]);
info("got event: " + gotEvent);
if (gotEvent) {
waitForPopupPositioned(() => {}, callback);
} else {
SimpleTest.executeSoon(callback);
}
}
function _openPopup(position, callback) {

View File

@ -95,7 +95,7 @@
return this.getAttribute("type") == "arrow";
}
adjustArrowPosition() {
adjustArrowPosition(event) {
if (!this.isArrowPanel || !this.isAnchored) {
return;
}
@ -103,8 +103,8 @@
var container = this.shadowRoot.querySelector(".panel-arrowcontainer");
var arrowbox = this.shadowRoot.querySelector(".panel-arrowbox");
var position = this.alignmentPosition;
var offset = this.alignmentOffset;
var position = event.alignmentPosition;
var offset = event.alignmentOffset;
this.setAttribute("arrowposition", position);
@ -296,7 +296,7 @@
on_popuppositioned(event) {
if (event.target == this) {
this.adjustArrowPosition();
this.adjustArrowPosition(event);
}
}
}

View File

@ -902,7 +902,7 @@ class WidgetEvent : public WidgetEventTime {
mMessage == eMouseOut || mMessage == eMouseMove ||
mMessage == eContextMenu || mMessage == eXULPopupShowing ||
mMessage == eXULPopupHiding || mMessage == eXULPopupShown ||
mMessage == eXULPopupHidden || mMessage == eXULPopupPositioned;
mMessage == eXULPopupHidden;
break;
case ePointerEventClass:
// All pointer events are composed