Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 0c30004

Browse files
masayuki-nakanomasayuki-nakano
committed
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
1 parent 6f372ca commit 0c30004

6 files changed

Lines changed: 184 additions & 22 deletions

File tree

‎devtools/client/inspector/test/shared-head.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,22 @@ var focusEditableField = async function(ruleView, editable, xOffset = 1,
383383
yOffset = 1, options = {}) {
384384
const onFocus = once(editable.parentNode, "focus", true);
385385
info("Clicking on editable field to turn to edit mode");
386-
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
387-
editable.ownerDocument.defaultView);
386+
if (options.type === undefined) {
387+
// "mousedown" and "mouseup" flushes any pending layout. Therefore,
388+
// if the caller wants to click an element, e.g., closebrace to add new
389+
// property, we need to guarantee that the element is clicked here even
390+
// if it's moved by flushing the layout because whether the UI is useful
391+
// or not when there is pending reflow is not scope of the tests.
392+
options.type = "mousedown";
393+
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
394+
editable.ownerGlobal);
395+
options.type = "mouseup";
396+
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
397+
editable.ownerGlobal);
398+
} else {
399+
EventUtils.synthesizeMouse(editable, xOffset, yOffset, options,
400+
editable.ownerGlobal);
401+
}
388402
await onFocus;
389403

390404
info("Editable field gained focus, returning the input field now");

‎dom/events/test/mochitest.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ subsuite = clipboard
153153
[test_bug1514940.html]
154154
skip-if = !debug
155155
[test_click_on_reframed_generated_text.html]
156+
[test_click_on_restyled_element.html]
156157
[test_clickevent_on_input.html]
157158
skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
158159
[test_continuous_wheel_events.html]

‎dom/events/test/test_click_on_restyled_element.html‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE HTML>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Test for clicking on an element which is restyled/reframed by mousedown event</title>
6+
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
7+
<script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
8+
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
9+
<style>
10+
.before-pseudo-element *:active::before {
11+
content: "";
12+
display: block;
13+
height: 2px;
14+
position: absolute;
15+
top: -2px;
16+
left: 0;
17+
width: 100%;
18+
}
19+
.position-relative *:active {
20+
position: relative;
21+
top: 1px;
22+
}
23+
</style>
24+
</head>
25+
<body>
26+
<section class="before-pseudo-element"><a href="about:blank">link</a></section><!-- bug 1398196 -->
27+
<section class="before-pseudo-element"><span>span</span></section>
28+
<section class="position-relative"><a href="about:blank">link</a></section><!-- bug 1506508 -->
29+
<section class="position-relative"><span>span</span></section>
30+
<script type="application/javascript">
31+
SimpleTest.waitForExplicitFinish();
32+
SimpleTest.waitForFocus(function doTest() {
33+
for (let sectionId of ["before-pseudo-element", "position-relative"]) {
34+
for (let element of ["a", "span"]) {
35+
let target = document.querySelector(`section.${sectionId} ${element}`);
36+
target.scrollIntoView(true);
37+
let clicked = false;
38+
target.addEventListener("click", (aEvent) => {
39+
is(aEvent.target, target, `click event is fired on the <${element}> element in ${sectionId} section as expected`);
40+
aEvent.preventDefault();
41+
clicked = true;
42+
}, {once: true});
43+
synthesizeMouseAtCenter(target, {});
44+
ok(clicked, `Click event should've been fired on the <${element}> element in ${sectionId} section`);
45+
}
46+
}
47+
SimpleTest.finish();
48+
});
49+
</script>
50+
</body>
51+
</html>

‎layout/base/PresShell.cpp‎

Lines changed: 90 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6506,6 +6506,16 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
65066506
}
65076507

65086508
if (aGUIEvent->IsUsingCoordinates()) {
6509+
// Flush pending notifications to handle the event with the latest layout.
6510+
// But if it causes destroying the frame for mPresShell, stop handling the
6511+
// event. (why?)
6512+
AutoWeakFrame weakFrame(aFrame);
6513+
MaybeFlushPendingNotifications(aGUIEvent);
6514+
if (!weakFrame.IsAlive()) {
6515+
*aEventStatus = nsEventStatus_eIgnore;
6516+
return NS_OK;
6517+
}
6518+
65096519
// XXX Retrieving capturing content here. However, some of the following
65106520
// methods allow to run script. So, isn't it possible the capturing
65116521
// content outdated?
@@ -6601,18 +6611,11 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
66016611
frameToHandleEvent = TouchManager::SetupTarget(
66026612
aGUIEvent->AsTouchEvent(), rootFrameToHandleEvent);
66036613
} else {
6604-
uint32_t flags = 0;
6605-
nsPoint eventPoint = nsLayoutUtils::GetEventCoordinatesRelativeTo(
6606-
aGUIEvent, rootFrameToHandleEvent);
6607-
6608-
if (mouseEvent && mouseEvent->mClass == eMouseEventClass &&
6609-
mouseEvent->mIgnoreRootScrollFrame) {
6610-
flags |= INPUT_IGNORE_ROOT_SCROLL_FRAME;
6611-
}
6612-
nsIFrame* target = FindFrameTargetedByInputEvent(
6613-
aGUIEvent, rootFrameToHandleEvent, eventPoint, flags);
6614-
if (target) {
6615-
frameToHandleEvent = target;
6614+
frameToHandleEvent =
6615+
GetFrameToHandleNonTouchEvent(rootFrameToHandleEvent, aGUIEvent);
6616+
if (!frameToHandleEvent) {
6617+
*aEventStatus = nsEventStatus_eIgnore;
6618+
return NS_OK;
66166619
}
66176620
}
66186621
}
@@ -6941,6 +6944,81 @@ nsresult PresShell::EventHandler::HandleEvent(nsIFrame* aFrame,
69416944
return rv;
69426945
}
69436946

6947+
bool PresShell::EventHandler::MaybeFlushPendingNotifications(
6948+
WidgetGUIEvent* aGUIEvent) {
6949+
MOZ_ASSERT(aGUIEvent);
6950+
6951+
switch (aGUIEvent->mMessage) {
6952+
case eMouseDown:
6953+
case eMouseUp: {
6954+
RefPtr<nsPresContext> presContext = mPresShell->GetPresContext();
6955+
if (NS_WARN_IF(!presContext)) {
6956+
return false;
6957+
}
6958+
uint64_t framesConstructedCount = presContext->FramesConstructedCount();
6959+
uint64_t framesReflowedCount = presContext->FramesReflowedCount();
6960+
6961+
mPresShell->FlushPendingNotifications(FlushType::Layout);
6962+
return framesConstructedCount != presContext->FramesConstructedCount() ||
6963+
framesReflowedCount != presContext->FramesReflowedCount();
6964+
}
6965+
default:
6966+
return false;
6967+
}
6968+
}
6969+
6970+
nsIFrame* PresShell::EventHandler::GetFrameToHandleNonTouchEvent(
6971+
nsIFrame* aRootFrameToHandleEvent, WidgetGUIEvent* aGUIEvent) {
6972+
MOZ_ASSERT(aGUIEvent);
6973+
MOZ_ASSERT(aGUIEvent->mClass != eTouchEventClass);
6974+
6975+
nsPoint eventPoint = nsLayoutUtils::GetEventCoordinatesRelativeTo(
6976+
aGUIEvent, aRootFrameToHandleEvent);
6977+
6978+
uint32_t flags = 0;
6979+
if (aGUIEvent->mClass == eMouseEventClass) {
6980+
WidgetMouseEvent* mouseEvent = aGUIEvent->AsMouseEvent();
6981+
if (mouseEvent && mouseEvent->mIgnoreRootScrollFrame) {
6982+
flags |= INPUT_IGNORE_ROOT_SCROLL_FRAME;
6983+
}
6984+
}
6985+
6986+
nsIFrame* targetFrame = FindFrameTargetedByInputEvent(
6987+
aGUIEvent, aRootFrameToHandleEvent, eventPoint, flags);
6988+
if (!targetFrame) {
6989+
return aRootFrameToHandleEvent;
6990+
}
6991+
6992+
if (targetFrame->PresShell() == mPresShell) {
6993+
// If found target is in mPresShell, we've already found it in the latest
6994+
// layout so that we can use it.
6995+
return targetFrame;
6996+
}
6997+
6998+
// If target is in a child document, we've not flushed its layout yet.
6999+
PresShell* childPresShell = static_cast<PresShell*>(targetFrame->PresShell());
7000+
EventHandler childEventHandler(*childPresShell);
7001+
AutoWeakFrame weakFrame(aRootFrameToHandleEvent);
7002+
bool layoutChanged =
7003+
childEventHandler.MaybeFlushPendingNotifications(aGUIEvent);
7004+
if (!weakFrame.IsAlive()) {
7005+
// Stop handling the event if the root frame to handle event is destroyed
7006+
// by the reflow. (but why?)
7007+
return nullptr;
7008+
}
7009+
if (!layoutChanged) {
7010+
// If the layout in the child PresShell hasn't been changed, we don't
7011+
// need to recompute the target.
7012+
return targetFrame;
7013+
}
7014+
7015+
// Finally, we need to recompute the target with the latest layout.
7016+
targetFrame = FindFrameTargetedByInputEvent(
7017+
aGUIEvent, aRootFrameToHandleEvent, eventPoint, flags);
7018+
7019+
return targetFrame ? targetFrame : aRootFrameToHandleEvent;
7020+
}
7021+
69447022
bool PresShell::EventHandler::MaybeHandleEventWithAccessibleCaret(
69457023
WidgetGUIEvent* aGUIEvent, nsEventStatus* aEventStatus) {
69467024
MOZ_ASSERT(aGUIEvent);

‎layout/base/PresShell.h‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,32 @@ class PresShell final : public nsIPresShell,
581581
static already_AddRefed<nsIURI> GetDocumentURIToCompareWithBlacklist(
582582
PresShell& aPresShell);
583583

584+
/**
585+
* MaybeFlushPendingNotifications() maybe flush pending notifications if
586+
* aGUIEvent should be handled with the latest layout.
587+
*
588+
* @param aGUIEvent The handling event.
589+
* @return true if this actually flushes pending
590+
* layout and that has caused changing the
591+
* layout.
592+
*/
593+
MOZ_CAN_RUN_SCRIPT
594+
bool MaybeFlushPendingNotifications(WidgetGUIEvent* aGUIEvent);
595+
596+
/**
597+
* GetFrameToHandleNonTouchEvent() returns a frame to handle the event.
598+
* This may flush pending layout if the target is in child PresShell.
599+
*
600+
* @param aRootFrameToHandleEvent The root frame to handle the event.
601+
* @param aGUIEvent The handling event.
602+
* @return The frame which should handle the
603+
* event. nullptr if the caller should
604+
* stop handling the event.
605+
*/
606+
MOZ_CAN_RUN_SCRIPT
607+
nsIFrame* GetFrameToHandleNonTouchEvent(nsIFrame* aRootFrameToHandleEvent,
608+
WidgetGUIEvent* aGUIEvent);
609+
584610
/**
585611
* MaybeDiscardEvent() checks whether it's safe to handle aGUIEvent right
586612
* now. If it's not safe, this may notify somebody of discarding event if

‎view/nsViewManager.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -752,14 +752,6 @@ void nsViewManager::DispatchEvent(WidgetGUIEvent* aEvent, nsView* aView,
752752
// want to cause its destruction in, say, some JavaScript event handler.
753753
nsCOMPtr<nsIPresShell> shell = view->GetViewManager()->GetPresShell();
754754
if (shell) {
755-
if (aEvent->mMessage == eMouseDown || aEvent->mMessage == eMouseUp) {
756-
AutoWeakFrame weakFrame(frame);
757-
shell->FlushPendingNotifications(FlushType::Layout);
758-
if (!weakFrame.IsAlive()) {
759-
*aStatus = nsEventStatus_eIgnore;
760-
return;
761-
}
762-
}
763755
shell->HandleEvent(frame, aEvent, false, aStatus);
764756
return;
765757
}

0 commit comments

Comments
 (0)