Skip to content

GitLab

T
tangled
Commit 63880105 authored by Kevin McNee's avatar Kevin McNee Committed by Chromium LUCI CQ
Browse files
Options

Have webview loadcommit event use the correct URL 

The webview loadcommit event's url property is documented as being
"the URL that committed," however we are currently setting it to
whatever the webview's src attribute is set to.

We now set the event's url property correctly.

See https://developer.chrome.com/apps/tags/webview#event-loadcommit

Bug: 1149615
Change-Id: I7e00bc78a503141893b0414ee1c8a4f7e2eecde2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545264
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: default avatarJames MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841296}
parent a2607609
Hide whitespace changes
Inline Side-by-side
Showing with 47 additions and 6 deletions
extensions/browser/guest_view/web_view/web_view_apitest.cc
View file @ 63880105
...@@ -663,6 +663,13 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNavOnSrcAttributeChange) { ...@@ -663,6 +663,13 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNavOnSrcAttributeChange) {
RunTest("testNavOnSrcAttributeChange", "web_view/apitest"); RunTest("testNavOnSrcAttributeChange", "web_view/apitest");
} }
IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestLoadCommitUrlsWithIframe) {
const std::string app_location = "web_view/apitest";
StartTestServer(app_location);
RunTest("testLoadCommitUrlsWithIframe", app_location);
StopTestServer();
}
IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNewWindow) { IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNewWindow) {
std::string app_location = "web_view/apitest"; std::string app_location = "web_view/apitest";
StartTestServer(app_location); StartTestServer(app_location);
......
extensions/renderer/resources/guest_view/web_view/web_view_events.js
View file @ 63880105
...@@ -277,12 +277,6 @@ WebViewEvents.prototype.handleLoadCommitEvent = function(event, eventName) { ...@@ -277,12 +277,6 @@ WebViewEvents.prototype.handleLoadCommitEvent = function(event, eventName) {
event.processId, event.processId,
event.visibleUrl); event.visibleUrl);
// TODO(1149615): The |url| property of the loadcommit event is documented as
// being set to the committed URL, but was being incorrectly set. This
// preserves the existing incorrect behaviour which we'll defer fixing in
// case existing code relies on this behaviour.
event.url = event.visibleUrl;
var webViewEvent = this.makeDomEvent(event, eventName); var webViewEvent = this.makeDomEvent(event, eventName);
this.view.dispatchEvent(webViewEvent); this.view.dispatchEvent(webViewEvent);
}; };
......
extensions/test/data/web_view/apitest/empty_frame.html 0 → 100644
View file @ 63880105
<!doctype html>
<!--
* Copyright (c) 2020 The Chromium Authors. All rights reserved. Use of this
* source code is governed by a BSD-style license that can be found in the
* LICENSE file.
-->
<html>
<body>
<p>Hello</p>
</body>
</html>
extensions/test/data/web_view/apitest/main.js
View file @ 63880105
...@@ -12,6 +12,7 @@ embedder.setUp_ = function(config) { ...@@ -12,6 +12,7 @@ embedder.setUp_ = function(config) {
embedder.baseGuestURL = 'http://localhost:' + config.testServer.port; embedder.baseGuestURL = 'http://localhost:' + config.testServer.port;
embedder.closeSocketURL = embedder.baseGuestURL + '/close-socket'; embedder.closeSocketURL = embedder.baseGuestURL + '/close-socket';
embedder.emptyGuestURL = embedder.baseGuestURL + '/empty_guest.html'; embedder.emptyGuestURL = embedder.baseGuestURL + '/empty_guest.html';
embedder.emptyFrameURL = embedder.baseGuestURL + '/empty_frame.html';
embedder.noReferrerGuestURL = embedder.noReferrerGuestURL =
embedder.baseGuestURL + '/guest_noreferrer.html'; embedder.baseGuestURL + '/guest_noreferrer.html';
embedder.detectUserAgentURL = embedder.baseGuestURL + '/detect-user-agent'; embedder.detectUserAgentURL = embedder.baseGuestURL + '/detect-user-agent';
...@@ -1305,6 +1306,33 @@ function testNavOnSrcAttributeChange() { ...@@ -1305,6 +1306,33 @@ function testNavOnSrcAttributeChange() {
document.body.appendChild(webview); document.body.appendChild(webview);
} }
// Tests that loadcommit has the correct |url| set when a guest's iframe commits
// a navigation.
function testLoadCommitUrlsWithIframe() {
let webview = document.createElement('webview');
let loadCommitSeen = 0;
webview.addEventListener('loadcommit', function(e) {
++loadCommitSeen;
if (loadCommitSeen === 1) {
embedder.test.assertEq(embedder.emptyGuestURL, e.url);
embedder.test.assertEq(true, e.isTopLevel);
embedder.test.assertEq(embedder.emptyGuestURL, webview.src);
webview.executeScript({
code: "let iframe = document.createElement('iframe'); " +
"iframe.src = '" + embedder.emptyFrameURL + "'; " +
"document.body.appendChild(iframe);"
});
} else if (loadCommitSeen === 2) {
embedder.test.assertEq(embedder.emptyFrameURL, e.url);
embedder.test.assertEq(false, e.isTopLevel);
embedder.test.assertEq(embedder.emptyGuestURL, webview.src);
embedder.test.succeed();
}
});
webview.src = embedder.emptyGuestURL;
document.body.appendChild(webview);
}
// This test verifies that new window attachment functions as expected. // This test verifies that new window attachment functions as expected.
// //
// TODO(crbug.com/594215) Test that opening a new window with a data URL is // TODO(crbug.com/594215) Test that opening a new window with a data URL is
...@@ -1969,6 +1997,7 @@ embedder.test.testList = { ...@@ -1969,6 +1997,7 @@ embedder.test.testList = {
'testNavOnConsecutiveSrcAttributeChanges': 'testNavOnConsecutiveSrcAttributeChanges':
testNavOnConsecutiveSrcAttributeChanges, testNavOnConsecutiveSrcAttributeChanges,
'testNavOnSrcAttributeChange': testNavOnSrcAttributeChange, 'testNavOnSrcAttributeChange': testNavOnSrcAttributeChange,
'testLoadCommitUrlsWithIframe': testLoadCommitUrlsWithIframe,
'testNewWindow': testNewWindow, 'testNewWindow': testNewWindow,
'testNewWindowNoPreventDefault': testNewWindowNoPreventDefault, 'testNewWindowNoPreventDefault': testNewWindowNoPreventDefault,
'testNewWindowNoReferrerLink': testNewWindowNoReferrerLink, 'testNewWindowNoReferrerLink': testNewWindowNoReferrerLink,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment