Bug 1506508 - Make PresShell::EventHandler::HandleEvent() flush any dirty region before deciding to event target of eMouseDown and eMouseUp r=smaug

When preceding mouse event is handled, that may cause changing style of the
target.  Therefore, when an eMouseDown or eMouseUp event is handled, handlers
require the latest layout.  Currently, nsViewManager::DispatchEvent() tries
to guarantee that with calling nsIPresShell::FlushPendingNotifications()
with FlushType::Layout.  However, this just flushes the pending layout in
the PresShell associated with the nsViewManager instance.  I.e., if the
target is in a child PresShell, its layout hasn't been flushed.

This patch makes PresShell::EventHandler::HandleEvent() flush pending
notifications at first of handling events using coordinates (only when
eMouseDown or eMouseUp, though).  Then, when it realizes that the event
should be handled in a child PresShell, makes it flush its pending
notifications and then, recompute event target with the latest layout if
the layout is actually changed.

Differential Revision: https://phabricator.services.mozilla.com/D13720

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Masayuki Nakano 2019-02-11 23:25:45 +00:00
Родитель 6f372ca372
Коммит 0c300040e9
6 изменённых файлов: 184 добавлений и 22 удалений

Просмотреть файл

@ -383,8 +383,22 @@ var focusEditableField = async function(ruleView, editable, xOffset = 1,
yOffset = 1, options = {}) { yOffset = 1, options = {}) {
const onFocus = once(editable.parentNode, "focus", true); const onFocus = once(editable.parentNode, "focus", true);
info("Clicking on editable field to turn to edit mode"); info("Clicking on editable field to turn to edit mode");
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options, if (options.type === undefined) {
editable.ownerDocument.defaultView); // "mousedown" and "mouseup" flushes any pending layout. Therefore,
// if the caller wants to click an element, e.g., closebrace to add new
// property, we need to guarantee that the element is clicked here even
// if it's moved by flushing the layout because whether the UI is useful
// or not when there is pending reflow is not scope of the tests.
options.type = "mousedown";
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
editable.ownerGlobal);
options.type = "mouseup";
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
editable.ownerGlobal);
} else {
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
editable.ownerGlobal);
}
await onFocus; await onFocus;
info("Editable field gained focus, returning the input field now"); info("Editable field gained focus, returning the input field now");

Просмотреть файл

@ -153,6 +153,7 @@ subsuite = clipboard
[test_bug1514940.html] [test_bug1514940.html]
skip-if = !debug skip-if = !debug
[test_click_on_reframed_generated_text.html] [test_click_on_reframed_generated_text.html]
[test_click_on_restyled_element.html]
[test_clickevent_on_input.html] [test_clickevent_on_input.html]
skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
[test_continuous_wheel_events.html] [test_continuous_wheel_events.html]

Просмотреть файл

@ -0,0 +1,51 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<title>Test for clicking on an element which is restyled/reframed by mousedown event</title>
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
<style>
.before-pseudo-element *:active::before {
content: "";
display: block;
height: 2px;
position: absolute;
top: -2px;
left: 0;
width: 100%;
}
.position-relative *:active {
position: relative;
top: 1px;
}
</style>
</head>
<body>
<section class="before-pseudo-element"><a href="about:blank">link</a></section><!-- bug 1398196 -->
<section class="before-pseudo-element"><span>span</span></section>
<section class="position-relative"><a href="about:blank">link</a></section><!-- bug 1506508 -->
<section class="position-relative"><span>span</span></section>
<script type="application/javascript">
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(function doTest() {
for (let sectionId of ["before-pseudo-element", "position-relative"]) {
for (let element of ["a", "span"]) {
let target = document.querySelector(`section.${sectionId} ${element}`);
target.scrollIntoView(true);
let clicked = false;
target.addEventListener("click", (aEvent) => {
is(aEvent.target, target, `click event is fired on the <${element}> element in ${sectionId} section as expected`);
aEvent.preventDefault();
clicked = true;
}, {once: true});
synthesizeMouseAtCenter(target, {});
ok(clicked, `Click event should've been fired on the <${element}> element in ${sectionId} section`);
}
}
SimpleTest.finish();
});
</script>
</body>
</html>

Просмотреть файл

@ -6506,6 +6506,16 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
} }
if (aGUIEvent->IsUsingCoordinates()) { if (aGUIEvent->IsUsingCoordinates()) {
// Flush pending notifications to handle the event with the latest layout.
// But if it causes destroying the frame for mPresShell, stop handling the
// event. (why?)
AutoWeakFrame weakFrame(aFrame);
MaybeFlushPendingNotifications(aGUIEvent);
if (!weakFrame.IsAlive()) {
*aEventStatus = nsEventStatus_eIgnore;
return NS_OK;
}
// XXX Retrieving capturing content here. However, some of the following // XXX Retrieving capturing content here. However, some of the following
// methods allow to run script. So, isn't it possible the capturing // methods allow to run script. So, isn't it possible the capturing
// content outdated? // content outdated?
@ -6601,18 +6611,11 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
frameToHandleEvent = TouchManager::SetupTarget( frameToHandleEvent = TouchManager::SetupTarget(
aGUIEvent->AsTouchEvent(), rootFrameToHandleEvent); aGUIEvent->AsTouchEvent(), rootFrameToHandleEvent);
} else { } else {
uint32_t flags = 0; frameToHandleEvent =
nsPoint eventPoint = nsLayoutUtils::GetEventCoordinatesRelativeTo( GetFrameToHandleNonTouchEvent(rootFrameToHandleEvent, aGUIEvent);
aGUIEvent, rootFrameToHandleEvent); if (!frameToHandleEvent) {
*aEventStatus = nsEventStatus_eIgnore;
if (mouseEvent && mouseEvent->mClass == eMouseEventClass && return NS_OK;
mouseEvent->mIgnoreRootScrollFrame) {
flags |= INPUT_IGNORE_ROOT_SCROLL_FRAME;
}
nsIFrame* target = FindFrameTargetedByInputEvent(
aGUIEvent, rootFrameToHandleEvent, eventPoint, flags);
if (target) {
frameToHandleEvent = target;
} }
} }
} }
@ -6941,6 +6944,81 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
return rv; return rv;
} }
bool PresShell::EventHandler::MaybeFlushPendingNotifications(
WidgetGUIEvent* aGUIEvent) {
MOZ_ASSERT(aGUIEvent);
switch (aGUIEvent->mMessage) {
case eMouseDown:
case eMouseUp: {
RefPtr<nsPresContext> presContext = mPresShell->GetPresContext();
if (NS_WARN_IF(!presContext)) {
return false;
}
uint64_t framesConstructedCount = presContext->FramesConstructedCount();
uint64_t framesReflowedCount = presContext->FramesReflowedCount();
mPresShell->FlushPendingNotifications(FlushType::Layout);
return framesConstructedCount != presContext->FramesConstructedCount() ||
framesReflowedCount != presContext->FramesReflowedCount();
}
default:
return false;
}
}
nsIFrame* PresShell::EventHandler::GetFrameToHandleNonTouchEvent(
nsIFrame* aRootFrameToHandleEvent, WidgetGUIEvent* aGUIEvent) {
MOZ_ASSERT(aGUIEvent);
MOZ_ASSERT(aGUIEvent->mClass != eTouchEventClass);
nsPoint eventPoint = nsLayoutUtils::GetEventCoordinatesRelativeTo(
aGUIEvent, aRootFrameToHandleEvent);
uint32_t flags = 0;
if (aGUIEvent->mClass == eMouseEventClass) {
WidgetMouseEvent* mouseEvent = aGUIEvent->AsMouseEvent();
if (mouseEvent && mouseEvent->mIgnoreRootScrollFrame) {
flags |= INPUT_IGNORE_ROOT_SCROLL_FRAME;
}
}
nsIFrame* targetFrame = FindFrameTargetedByInputEvent(
aGUIEvent, aRootFrameToHandleEvent, eventPoint, flags);
if (!targetFrame) {
return aRootFrameToHandleEvent;
}
if (targetFrame->PresShell() == mPresShell) {
// If found target is in mPresShell, we've already found it in the latest
// layout so that we can use it.
return targetFrame;
}
// If target is in a child document, we've not flushed its layout yet.
PresShell* childPresShell = static_cast<PresShell*>(targetFrame->PresShell());
EventHandler childEventHandler(*childPresShell);
AutoWeakFrame weakFrame(aRootFrameToHandleEvent);
bool layoutChanged =
childEventHandler.MaybeFlushPendingNotifications(aGUIEvent);
if (!weakFrame.IsAlive()) {
// Stop handling the event if the root frame to handle event is destroyed
// by the reflow. (but why?)
return nullptr;
}
if (!layoutChanged) {
// If the layout in the child PresShell hasn't been changed, we don't
// need to recompute the target.
return targetFrame;
}
// Finally, we need to recompute the target with the latest layout.
targetFrame = FindFrameTargetedByInputEvent(
aGUIEvent, aRootFrameToHandleEvent, eventPoint, flags);
return targetFrame ? targetFrame : aRootFrameToHandleEvent;
}
bool PresShell::EventHandler::MaybeHandleEventWithAccessibleCaret( bool PresShell::EventHandler::MaybeHandleEventWithAccessibleCaret(
WidgetGUIEvent* aGUIEvent, nsEventStatus* aEventStatus) { WidgetGUIEvent* aGUIEvent, nsEventStatus* aEventStatus) {
MOZ_ASSERT(aGUIEvent); MOZ_ASSERT(aGUIEvent);

Просмотреть файл

@ -581,6 +581,32 @@ class PresShell final : public nsIPresShell,
static already_AddRefed<nsIURI> GetDocumentURIToCompareWithBlacklist( static already_AddRefed<nsIURI> GetDocumentURIToCompareWithBlacklist(
PresShell& aPresShell); PresShell& aPresShell);
/**
* MaybeFlushPendingNotifications() maybe flush pending notifications if
* aGUIEvent should be handled with the latest layout.
*
* @param aGUIEvent The handling event.
* @return true if this actually flushes pending
* layout and that has caused changing the
* layout.
*/
MOZ_CAN_RUN_SCRIPT
bool MaybeFlushPendingNotifications(WidgetGUIEvent* aGUIEvent);
/**
* GetFrameToHandleNonTouchEvent() returns a frame to handle the event.
* This may flush pending layout if the target is in child PresShell.
*
* @param aRootFrameToHandleEvent The root frame to handle the event.
* @param aGUIEvent The handling event.
* @return The frame which should handle the
* event. nullptr if the caller should
* stop handling the event.
*/
MOZ_CAN_RUN_SCRIPT
nsIFrame* GetFrameToHandleNonTouchEvent(nsIFrame* aRootFrameToHandleEvent,
WidgetGUIEvent* aGUIEvent);
/** /**
* MaybeDiscardEvent() checks whether it's safe to handle aGUIEvent right * MaybeDiscardEvent() checks whether it's safe to handle aGUIEvent right
* now. If it's not safe, this may notify somebody of discarding event if * now. If it's not safe, this may notify somebody of discarding event if

Просмотреть файл

@ -752,14 +752,6 @@ void nsViewManager::DispatchEvent(WidgetGUIEvent* aEvent, nsView* aView,
// want to cause its destruction in, say, some JavaScript event handler. // want to cause its destruction in, say, some JavaScript event handler.
nsCOMPtr<nsIPresShell> shell = view->GetViewManager()->GetPresShell(); nsCOMPtr<nsIPresShell> shell = view->GetViewManager()->GetPresShell();
if (shell) { if (shell) {
if (aEvent->mMessage == eMouseDown || aEvent->mMessage == eMouseUp) {
AutoWeakFrame weakFrame(frame);
shell->FlushPendingNotifications(FlushType::Layout);
if (!weakFrame.IsAlive()) {
*aStatus = nsEventStatus_eIgnore;
return;
}
}
shell->HandleEvent(frame, aEvent, false, aStatus); shell->HandleEvent(frame, aEvent, false, aStatus);
return; return;
} }