Skip to content

Commit 503f473

Browse files
jesxilinjesxilin
committed
dragend event has incorrect coordinates in a nested <iframe>
https://bugs.webkit.org/show_bug.cgi?id=308617 rdar://170750013 Reviewed by Aditya Keerthi. The dragEnd event coordinates are off by a substantial amount while the dragStart event coordinates are correct. This is because when WebKit receives the end point in WebViewImpl::sendDragEndToPage from AppKit, all WebKit does is convert this point from screen to window coordinates. The coordinates of clientX/clientY are supposed to be in respect to the content space of the frame that the events are on. Previously, a frame (content space) was not specified, so after receiving screen to window coordinates, WebKit just converts to main frame content coordinates. To fix this issue, first do an additional step and convert from window coordinates to WKWebView coordinates. Since AppKit gives us the end point, the drag offset does not need to be applied in WebPage::dragEnded. In EventHandler::dispatchEventToDragSourceElement, use the dragged element's frame to call dispatchDragEvent. This is the frame that has the dragStart and dragEnd event. Now, dispatchDragEvent will be on the correct frame's EventHandler. From here, it is then converted from WKWebView coordinates to the correct frame's content coordinates, yielding the correct clientX/clientY. Add an API test that checks for dragEnd coordinates when dragging within the nested iframe as well as dragging to outside the iframe. Update DragAndDropTests.DragSourceEndedAtCoordinateTransformation as the expected coordinates were previously incorrect. * Source/WebCore/page/EventHandler.cpp: (WebCore::EventHandler::dispatchEventToDragSourceElement): * Source/WebKit/UIProcess/mac/WebViewImpl.mm: (WebKit::WebViewImpl::sendDragEndToPage): * Source/WebKit/WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::dragEnded): * Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm: (TestWebKitAPI::(SiteIsolation, DragSourceEndedAtCoordinateTransformation)): * Tools/TestWebKitAPI/Tests/mac/DragAndDropTestsMac.mm: (TEST(DragAndDropTests, DragEndEventCoordinatesWithNestedIframes)): Canonical link: https://commits.webkit.org/308216@main
1 parent e869783 commit 503f473

File tree

5 files changed

+127
-16
lines changed

5 files changed

+127
-16
lines changed

‎Source/WebCore/page/EventHandler.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4649,8 +4649,8 @@ bool EventHandler::shouldDispatchEventsToDragSourceElement()
46494649

46504650
void EventHandler::dispatchEventToDragSourceElement(const AtomString& eventType, const PlatformMouseEvent& event)
46514651
{
4652-
if (shouldDispatchEventsToDragSourceElement())
4653-
dispatchDragEvent(eventType, *protect(draggedElement()), event, *dragState().dataTransfer);
4652+
if (RefPtr frame = draggedElement()->document().frame(); frame && shouldDispatchEventsToDragSourceElement())
4653+
frame->eventHandler().dispatchDragEvent(eventType, *protect(draggedElement()), event, *dragState().dataTransfer);
46544654
}
46554655

46564656
bool EventHandler::dispatchDragStartEventOnSourceElement(DataTransfer& dataTransfer)

‎Source/WebKit/UIProcess/mac/WebViewImpl.mm‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4319,7 +4319,10 @@ static String commandNameForSelector(SEL selector)
43194319
// Prevent queued mouseDragged events from coming after the drag and fake mouseUp event.
43204320
m_ignoresMouseDraggedEvents = true;
43214321

4322-
m_page->dragEnded(WebCore::IntPoint(windowMouseLoc), WebCore::IntPoint(WebCore::globalPoint(windowMouseLoc, protect(window()).get())), coreDragOperationMask(dragOperationMask));
4322+
RetainPtr view = m_view.get();
4323+
WebCore::IntPoint clientLocation([view convertPoint:windowImageLoc fromView:nil]);
4324+
4325+
m_page->dragEnded(clientLocation, WebCore::IntPoint(WebCore::globalPoint(windowMouseLoc, protect(window()).get())), coreDragOperationMask(dragOperationMask));
43234326
}
43244327

43254328
static OptionSet<WebCore::DragApplicationFlags> applicationFlagsForDrag(NSView *view, id<NSDraggingInfo> draggingInfo)

‎Source/WebKit/WebProcess/WebPage/WebPage.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5598,7 +5598,6 @@ void WebPage::performDragOperation(std::optional<WebCore::FrameIdentifier> frame
55985598

55995599
void WebPage::dragEnded(std::optional<FrameIdentifier> frameID, IntPoint clientPosition, IntPoint globalPosition, OptionSet<DragOperation> dragOperationMask, CompletionHandler<void(std::optional<RemoteUserInputEventData>)>&& completionHandler)
56005600
{
5601-
IntPoint adjustedClientPosition(clientPosition.x() + m_page->dragController().dragOffset().x(), clientPosition.y() + m_page->dragController().dragOffset().y());
56025601
IntPoint adjustedGlobalPosition(globalPosition.x() + m_page->dragController().dragOffset().x(), globalPosition.y() + m_page->dragController().dragOffset().y());
56035602

56045603
m_page->dragController().dragEnded();
@@ -5615,7 +5614,7 @@ void WebPage::dragEnded(std::optional<FrameIdentifier> frameID, IntPoint clientP
56155614
return completionHandler(std::nullopt);
56165615

56175616
// FIXME: These are fake modifier keys here, but they should be real ones instead.
5618-
PlatformMouseEvent event(adjustedClientPosition, adjustedGlobalPosition, MouseButton::Left, PlatformEvent::Type::MouseMoved, 0, { }, MonotonicTime::now(), 0, WebCore::SyntheticClickType::NoTap, MouseEventInputSource::UserDriven);
5617+
PlatformMouseEvent event(clientPosition, adjustedGlobalPosition, MouseButton::Left, PlatformEvent::Type::MouseMoved, 0, { }, MonotonicTime::now(), 0, WebCore::SyntheticClickType::NoTap, MouseEventInputSource::UserDriven);
56195618
auto remoteUserInputEventData = localFrame->eventHandler().dragSourceEndedAt(event, dragOperationMask);
56205619

56215620
completionHandler(remoteUserInputEventData);

‎Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7169,25 +7169,25 @@ HTTPServer server({
71697169

71707170
RetainPtr navigationDelegate = adoptNS([TestNavigationDelegate new]);
71717171
[navigationDelegate allowAnyTLSCertificate];
7172-
auto configuration = server.httpsProxyConfiguration();
7172+
RetainPtr configuration = server.httpsProxyConfiguration();
71737173
enableSiteIsolation(configuration);
7174-
RetainPtr simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebViewFrame:NSMakeRect(0, 0, 600, 600) configuration:configuration]);
7174+
RetainPtr simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebViewFrame:NSMakeRect(0, 0, 600, 600) configuration:configuration.get()]);
71757175
RetainPtr webView = [simulator webView];
7176-
webView.get().navigationDelegate = navigationDelegate.get();
7176+
[webView setNavigationDelegate:navigationDelegate.get()];
71777177

71787178
[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://domain1.com/mainframe"]]];
71797179
[navigationDelegate waitForDidFinishNavigation];
71807180
[webView waitForNextPresentationUpdate];
71817181
[simulator runFrom:CGPointMake(300, 300) to:CGPointMake(350, 350)];
71827182

7183-
NSArray<NSString *> *events = [webView objectByEvaluatingJavaScript:@"window.events"];
7184-
EXPECT_GT(events.count, 0U);
7183+
RetainPtr<NSArray<NSString *>> events = [webView objectByEvaluatingJavaScript:@"window.events"];
7184+
EXPECT_GT([events count], 0U);
71857185

71867186
bool foundDragStart = false;
71877187
bool foundDragEnd = false;
71887188
NSString *dragEndEvent = nil;
71897189

7190-
for (NSString *event in events) {
7190+
for (NSString *event in events.get()) {
71917191
if ([event hasPrefix:@"dragstart:"]) {
71927192
foundDragStart = true;
71937193
} else if ([event hasPrefix:@"dragend:"]) {
@@ -7200,13 +7200,13 @@ HTTPServer server({
72007200
EXPECT_TRUE(foundDragEnd) << "Should have received dragend event in remote frame";
72017201

72027202
if (dragEndEvent) {
7203-
NSString *coords = [dragEndEvent substringFromIndex:[@"dragend:" length]];
7204-
NSArray *components = [coords componentsSeparatedByString:@","];
7205-
if (components.count == 2) {
7203+
RetainPtr coords = [dragEndEvent substringFromIndex:[@"dragend:" length]];
7204+
RetainPtr components = [coords componentsSeparatedByString:@","];
7205+
if ([components count] == 2) {
72067206
int x = [components[0] intValue];
72077207
int y = [components[1] intValue];
7208-
EXPECT_TRUE(x >= 190 && x <= 200) << "Expected dragend x coordinate around 196, got " << x;
7209-
EXPECT_TRUE(y >= 95 && y <= 105) << "Expected dragend y coordinate around 100, got " << y;
7208+
EXPECT_TRUE(x >= 144 && x <= 154) << "Expected dragend x coordinate around 148, got " << x;
7209+
EXPECT_TRUE(y >= 144 && y <= 154) << "Expected dragend y coordinate around 148, got " << y;
72107210
}
72117211
}
72127212
}

‎Tools/TestWebKitAPI/Tests/mac/DragAndDropTestsMac.mm‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@
2626
#import "config.h"
2727

2828
#import "DragAndDropSimulator.h"
29+
#import "HTTPServer.h"
2930
#import "InstanceMethodSwizzler.h"
3031
#import "PlatformUtilities.h"
3132
#import "TestDraggingInfo.h"
33+
#import "TestNavigationDelegate.h"
3234
#import <UniformTypeIdentifiers/UniformTypeIdentifiers.h>
3335
#import <WebCore/PasteboardCustomData.h>
3436
#import <WebKit/WKPreferencesPrivate.h>
@@ -60,6 +62,113 @@
6062
EXPECT_EQ(1U, numberOfValidItemsForDrop);
6163
}
6264

65+
TEST(DragAndDropTests, DragEndEventCoordinatesWithNestedIframes)
66+
{
67+
static constexpr ASCIILiteral mainframeHTML = "<iframe width='500' height='500' style='position: absolute; top: 50px; left: 50px; border: 2px solid red;' src='https://domain2.com/subframe'></iframe>"_s;
68+
69+
static constexpr ASCIILiteral subframeHTML = "<script>"
70+
" window.events = [];"
71+
" addEventListener('message', function(event) {"
72+
" console.log(event.data);"
73+
" window.events.push(event.data);"
74+
" });"
75+
"</script>"
76+
"<iframe width='500' height='500' style='position: absolute; top: 50px; left: 50px; border: 2px solid blue;' src='https://domain3.com/nestedSubframe'></iframe>"_s;
77+
78+
static constexpr ASCIILiteral nestedSubframeHTML =
79+
"<body style='margin: 0; padding: 0; width: 100%; height: 100vh; background-color: lightblue;'>"
80+
"<div id='draggable' draggable='true' style='width: 100px; height: 100px; background-color: yellow; position: absolute; top: 50px; left: 50px;'>Drag me</div>"
81+
"<script>"
82+
"const draggable = document.getElementById('draggable');"
83+
"draggable.addEventListener('dragstart', (event) => {"
84+
"parent.postMessage('dragstart:' + event.clientX + ',' + event.clientY, '*');"
85+
"});"
86+
"draggable.addEventListener('dragend', (event) => {"
87+
"parent.postMessage('dragend:' + event.clientX + ',' + event.clientY, '*');"
88+
"});"
89+
"</script>"
90+
"</body>"_s;
91+
92+
TestWebKitAPI::HTTPServer server({
93+
{ "/mainframe"_s, { mainframeHTML } },
94+
{ "/subframe"_s, { subframeHTML } },
95+
{ "/nestedSubframe"_s, { nestedSubframeHTML } }
96+
}, TestWebKitAPI::HTTPServer::Protocol::HttpsProxy);
97+
98+
RetainPtr navigationDelegate = adoptNS([TestNavigationDelegate new]);
99+
[navigationDelegate allowAnyTLSCertificate];
100+
RetainPtr configuration = server.httpsProxyConfiguration();
101+
RetainPtr simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebViewFrame:NSMakeRect(0, 0, 800, 800) configuration:configuration.get()]);
102+
RetainPtr webView = [simulator webView];
103+
[webView setNavigationDelegate:navigationDelegate.get()];
104+
105+
[webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://domain1.com/mainframe"]]];
106+
[navigationDelegate waitForDidFinishNavigation];
107+
[webView waitForNextPresentationUpdate];
108+
[simulator runFrom:CGPointMake(200, 200) to:CGPointMake(300, 300)];
109+
110+
RetainPtr<NSArray<NSString *>> events = [webView objectByEvaluatingJavaScript:@"window.events" inFrame:[webView firstChildFrame]];
111+
EXPECT_GT([events count], 0U);
112+
bool foundDragStart = false;
113+
bool foundDragEnd = false;
114+
NSString *dragEndEvent = nil;
115+
116+
for (NSString *event in events.get()) {
117+
if ([event hasPrefix:@"dragstart:"]) {
118+
foundDragStart = true;
119+
} else if ([event hasPrefix:@"dragend:"]) {
120+
foundDragEnd = true;
121+
dragEndEvent = event;
122+
}
123+
}
124+
125+
EXPECT_TRUE(foundDragStart);
126+
EXPECT_TRUE(foundDragEnd);
127+
128+
if (dragEndEvent) {
129+
RetainPtr coords = [dragEndEvent substringFromIndex:[@"dragend:" length]];
130+
RetainPtr components = [coords componentsSeparatedByString:@","];
131+
if ([components count] == 2) {
132+
int x = [components[0] intValue];
133+
int y = [components[1] intValue];
134+
EXPECT_TRUE(x >= 190 && x <= 200) << "Expected dragend x coordinate around 193, got " << x;
135+
EXPECT_TRUE(y >= 190 && y <= 200) << "Expected dragend y coordinate around 196, got " << y;
136+
}
137+
}
138+
139+
[webView waitForNextPresentationUpdate];
140+
[simulator runFrom:CGPointMake(200, 200) to:CGPointMake(0, 0)];
141+
142+
events = [webView objectByEvaluatingJavaScript:@"window.events" inFrame:[webView firstChildFrame]];
143+
EXPECT_GT([events count], 0U);
144+
foundDragStart = false;
145+
foundDragEnd = false;
146+
dragEndEvent = nil;
147+
148+
for (NSString *event in events.get()) {
149+
if ([event hasPrefix:@"dragstart:"]) {
150+
foundDragStart = true;
151+
} else if ([event hasPrefix:@"dragend:"]) {
152+
foundDragEnd = true;
153+
dragEndEvent = event;
154+
}
155+
}
156+
157+
EXPECT_TRUE(foundDragStart);
158+
EXPECT_TRUE(foundDragEnd);
159+
160+
if (dragEndEvent) {
161+
RetainPtr coords = [dragEndEvent substringFromIndex:[@"dragend:" length]];
162+
RetainPtr components = [coords componentsSeparatedByString:@","];
163+
if ([components count] == 2) {
164+
int x = [components[0] intValue];
165+
int y = [components[1] intValue];
166+
EXPECT_TRUE(x >= -105 && x <= -95) << "Expected dragend x coordinate around -100, got " << x;
167+
EXPECT_TRUE(y >= -105 && y <= -95) << "Expected dragend y coordinate around -100, got " << y;
168+
}
169+
}
170+
}
171+
63172
TEST(DragAndDropTests, DropUserSelectAllUserDragElementDiv)
64173
{
65174
auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebViewFrame:NSMakeRect(0, 0, 320, 500)]);

0 commit comments

Comments
 (0)