From 74372d65aec2fa039487e07b0a9e8c4b1d33f2e1 Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Thu, 26 Mar 2020 19:05:45 +0100 Subject: [PATCH] feat: enhance native window.open to match the custom implementation's behavior (#19703) Co-authored-by: Andy Locascio --- docs/api/structures/post-body.md | 23 ++++ docs/api/structures/post-data.md | 21 +++ docs/api/web-contents.md | 17 ++- filenames.auto.gni | 2 + lib/browser/api/net.js | 1 - lib/browser/api/web-contents.js | 35 ++--- lib/browser/guest-view-manager.js | 13 +- lib/browser/guest-window-manager.js | 114 +++++++--------- lib/common/parse-features-string.js | 66 ++++++++- package.json | 2 +- patches/chromium/can_create_window.patch | 125 ++++++++++++++++-- shell/browser/api/electron_api_app.cc | 4 +- shell/browser/api/electron_api_app.h | 2 +- .../browser/api/electron_api_web_contents.cc | 38 +++--- shell/browser/api/electron_api_web_contents.h | 15 ++- shell/browser/child_web_contents_tracker.cc | 2 + shell/browser/child_web_contents_tracker.h | 5 + shell/browser/electron_browser_client.cc | 4 +- shell/browser/electron_browser_client.h | 2 +- spec-main/internal-spec.ts | 2 +- 20 files changed, 350 insertions(+), 143 deletions(-) create mode 100644 docs/api/structures/post-body.md create mode 100644 docs/api/structures/post-data.md diff --git a/docs/api/structures/post-body.md b/docs/api/structures/post-body.md new file mode 100644 index 0000000000000..baa479dda3f05 --- /dev/null +++ b/docs/api/structures/post-body.md @@ -0,0 +1,23 @@ +# PostBody Object + +* `data` Array<[PostData](./post-data.md)> - The post data to be sent to the + new window. +* `contentType` String - The `content-type` header used for the data. One of + `application/x-www-form-urlencoded` or `multipart/form-data`. Corresponds to + the `enctype` attribute of the submitted HTML form. +* `boundary` String (optional) - The boundary used to separate multiple parts of + the message. Only valid when `contentType` is `multipart/form-data`. + +Note that keys starting with `--` are not currently supported. For example, this will errantly submit as `multipart/form-data` when `nativeWindowOpen` is set to `false` in webPreferences: + +```html +
+ + +
+``` diff --git a/docs/api/structures/post-data.md b/docs/api/structures/post-data.md new file mode 100644 index 0000000000000..b63f20a95051b --- /dev/null +++ b/docs/api/structures/post-data.md @@ -0,0 +1,21 @@ +# PostData Object + +* `type` String - One of the following: + * `rawData` - The data is available as a `Buffer`, in the `rawData` field. + * `file` - The object represents a file. The `filePath`, `offset`, `length` + and `modificationTime` fields will be used to describe the file. + * `blob` - The object represents a `Blob`. The `blobUUID` field will be used + to describe the `Blob`. +* `bytes` String (optional) - The raw bytes of the post data in a `Buffer`. + Required for the `rawData` type. +* `filePath` String (optional) - The path of the file being uploaded. Required + for the `file` type. +* `blobUUID` String (optional) - The `UUID` of the `Blob` being uploaded. + Required for the `blob` type. +* `offset` Integer (optional) - The offset from the beginning of the file being + uploaded, in bytes. Only valid for `file` types. +* `length` Integer (optional) - The length of the file being uploaded, in bytes. + If set to `-1`, the whole file will be uploaded. Only valid for `file` types. +* `modificationTime` Double (optional) - The modification time of the file + represented by a double, which is the number of seconds since the `UNIX Epoch` + (Jan 1, 1970). Only valid for `file` types. diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index a44cdf279f6e8..8f952590ba41e 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -150,6 +150,10 @@ Returns: * `referrer` [Referrer](structures/referrer.md) - The referrer that will be passed to the new window. May or may not result in the `Referer` header being sent, depending on the referrer policy. +* `postBody` [PostBody](structures/post-body.md) (optional) - The post data that + will be sent to the new window, along with the appropriate headers that will + be set. If no post data is to be sent, the value will be `null`. Only defined + when the window is being created by a form that set `target=_blank`. Emitted when the page requests to open a new window for a `url`. It could be requested by `window.open` or an external link like ``. @@ -162,7 +166,7 @@ new [`BrowserWindow`](browser-window.md). If you call `event.preventDefault()` a instance, failing to do so may result in unexpected behavior. For example: ```javascript -myBrowserWindow.webContents.on('new-window', (event, url, frameName, disposition, options) => { +myBrowserWindow.webContents.on('new-window', (event, url, frameName, disposition, options, additionalFeatures, referrer, postBody) => { event.preventDefault() const win = new BrowserWindow({ webContents: options.webContents, // use existing webContents if provided @@ -170,7 +174,16 @@ myBrowserWindow.webContents.on('new-window', (event, url, frameName, disposition }) win.once('ready-to-show', () => win.show()) if (!options.webContents) { - win.loadURL(url) // existing webContents will be navigated automatically + const loadOptions = { + httpReferrer: referrer + } + if (postBody != null) { + const { data, contentType, boundary } = postBody + loadOptions.postData = postBody.data + loadOptions.extraHeaders = `content-type: ${contentType}; boundary=${boundary}` + } + + win.loadURL(url, loadOptions) // existing webContents will be navigated automatically } event.newGuest = win }) diff --git a/filenames.auto.gni b/filenames.auto.gni index 5041ad522e2c0..5588dc2f55388 100644 --- a/filenames.auto.gni +++ b/filenames.auto.gni @@ -102,6 +102,8 @@ auto_filenames = { "docs/api/structures/mouse-wheel-input-event.md", "docs/api/structures/notification-action.md", "docs/api/structures/point.md", + "docs/api/structures/post-body.md", + "docs/api/structures/post-data.md", "docs/api/structures/printer-info.md", "docs/api/structures/process-memory-info.md", "docs/api/structures/process-metric.md", diff --git a/lib/browser/api/net.js b/lib/browser/api/net.js index a469248a52801..54eb2f8d3e232 100644 --- a/lib/browser/api/net.js +++ b/lib/browser/api/net.js @@ -126,7 +126,6 @@ class SlurpStream extends Writable { this._data = Buffer.concat([this._data, chunk]); callback(); } - data () { return this._data; } } diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index bedc202c1ecaa..53394408fdbb3 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -11,6 +11,7 @@ const { internalWindowOpen } = require('@electron/internal/browser/guest-window- const NavigationController = require('@electron/internal/browser/navigation-controller'); const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal'); const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils'); +const { convertFeaturesString } = require('@electron/internal/common/parse-features-string'); const { MessagePortMain } = require('@electron/internal/browser/message-port-main'); // session is not used here, the purpose is to make sure session is initalized @@ -506,40 +507,42 @@ WebContents.prototype._init = function () { this.reload(); }); - // Handle window.open for BrowserWindow and BrowserView. - if (['browserView', 'window'].includes(this.getType())) { + if (this.getType() !== 'remote') { // Make new windows requested by links behave like "window.open". this.on('-new-window', (event, url, frameName, disposition, - additionalFeatures, postData, - referrer) => { - const options = { + rawFeatures, referrer, postData) => { + const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName); + const mergedOptions = { show: true, width: 800, - height: 600 + height: 600, + ...options }; - internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures, postData); + + internalWindowOpen(event, url, referrer, frameName, disposition, mergedOptions, additionalFeatures, postData); }); // Create a new browser window for the native implementation of // "window.open", used in sandbox and nativeWindowOpen mode. this.on('-add-new-contents', (event, webContents, disposition, - userGesture, left, top, width, height, url, frameName) => { + userGesture, left, top, width, height, url, frameName, + referrer, rawFeatures, postData) => { if ((disposition !== 'foreground-tab' && disposition !== 'new-window' && disposition !== 'background-tab')) { event.preventDefault(); return; } - const options = { + const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName); + const mergedOptions = { show: true, - x: left, - y: top, - width: width || 800, - height: height || 600, - webContents + width: 800, + height: 600, + webContents, + ...options }; - const referrer = { url: '', policy: 'default' }; - internalWindowOpen(event, url, referrer, frameName, disposition, options); + + internalWindowOpen(event, url, referrer, frameName, disposition, mergedOptions, additionalFeatures, postData); }); } diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index b27a43bba71b6..9e44432b4eae1 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -3,7 +3,7 @@ const { webContents } = require('electron'); const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal'); const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils'); -const parseFeaturesString = require('@electron/internal/common/parse-features-string'); +const { parseFeaturesString } = require('@electron/internal/common/parse-features-string'); const { syncMethods, asyncMethods, properties } = require('@electron/internal/common/web-view-methods'); const { serialize } = require('@electron/internal/common/type-utils'); @@ -141,17 +141,6 @@ const createGuest = function (embedder, params) { } }); - // Forward internal web contents event to embedder to handle - // native window.open setup - guest.on('-add-new-contents', (...args) => { - if (guest.getLastWebPreferences().nativeWindowOpen === true) { - const embedder = getEmbedder(guestInstanceId); - if (embedder != null) { - embedder.emit('-add-new-contents', ...args); - } - } - }); - return guestInstanceId; }; diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index b9aaaa6ce5bc2..bcef78ec48416 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -5,7 +5,7 @@ const { BrowserWindow } = electron; const { isSameOrigin } = process.electronBinding('v8_util'); const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal'); const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils'); -const parseFeaturesString = require('@electron/internal/common/parse-features-string'); +const { convertFeaturesString } = require('@electron/internal/common/parse-features-string'); const hasProp = {}.hasOwnProperty; const frameToGuest = new Map(); @@ -57,7 +57,11 @@ const mergeBrowserWindowOptions = function (embedder, options) { // if parent's visibility is available, that overrides 'show' flag (#12125) const win = BrowserWindow.fromWebContents(embedder.webContents); if (win != null) { - parentOptions = { ...embedder.browserWindowOptions, show: win.isVisible() }; + parentOptions = { + ...win.getBounds(), + ...embedder.browserWindowOptions, + show: win.isVisible() + }; } // Inherit the original options if it is a BrowserWindow. @@ -83,6 +87,40 @@ const mergeBrowserWindowOptions = function (embedder, options) { return options; }; +const MULTIPART_CONTENT_TYPE = 'multipart/form-data'; +const URL_ENCODED_CONTENT_TYPE = 'application/x-www-form-urlencoded'; +function makeContentTypeHeader ({ contentType, boundary }) { + const header = `content-type: ${contentType};`; + if (contentType === MULTIPART_CONTENT_TYPE) { + return `${header} boundary=${boundary}`; + } + return header; +} + +// Figure out appropriate headers for post data. +const parseContentTypeFormat = function (postData) { + if (postData.length) { + // For multipart forms, the first element will start with the boundary + // notice, which looks something like `------WebKitFormBoundary12345678` + // Note, this regex would fail when submitting a urlencoded form with an + // input attribute of name="--theKey", but, uhh, don't do that? + const postDataFront = postData[0].bytes.toString(); + const boundary = /^--.*[^-\r\n]/.exec(postDataFront); + if (boundary) { + return { + boundary: boundary[0].substr(2), + contentType: MULTIPART_CONTENT_TYPE + }; + } + } + // Either the form submission didn't contain any inputs (the postData array + // was empty), or we couldn't find the boundary and thus we can assume this is + // a key=value style form. + return { + contentType: URL_ENCODED_CONTENT_TYPE + }; +}; + // Setup a new guest with |embedder| const setupGuest = function (embedder, frameName, guest, options) { // When |embedder| is destroyed we should also destroy attached guest, and if @@ -134,14 +172,7 @@ const createGuest = function (embedder, url, referrer, frameName, options, postD }; if (postData != null) { loadOptions.postData = postData; - loadOptions.extraHeaders = 'content-type: application/x-www-form-urlencoded'; - if (postData.length > 0) { - const postDataFront = postData[0].bytes.toString(); - const boundary = /^--.*[^-\r\n]/.exec(postDataFront); - if (boundary != null) { - loadOptions.extraHeaders = `content-type: multipart/form-data; boundary=${boundary[0].substr(2)}`; - } - } + loadOptions.extraHeaders = makeContentTypeHeader(parseContentTypeFormat(postData)); } guest.loadURL(url, loadOptions); } @@ -192,68 +223,21 @@ ipcMainInternal.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', (event, url, fra if (frameName == null) frameName = ''; if (features == null) features = ''; - const options = {}; - - const ints = ['x', 'y', 'width', 'height', 'minWidth', 'maxWidth', 'minHeight', 'maxHeight', 'zoomFactor']; - const webPreferences = ['zoomFactor', 'nodeIntegration', 'enableRemoteModule', 'preload', 'javascript', 'contextIsolation', 'webviewTag']; const disposition = 'new-window'; - - // Used to store additional features - const additionalFeatures = []; - - // Parse the features - parseFeaturesString(features, function (key, value) { - if (value === undefined) { - additionalFeatures.push(key); - } else { - // Don't allow webPreferences to be set since it must be an object - // that cannot be directly overridden - if (key === 'webPreferences') return; - - if (webPreferences.includes(key)) { - if (options.webPreferences == null) { - options.webPreferences = {}; - } - options.webPreferences[key] = value; - } else { - options[key] = value; - } - } - }); - if (options.left) { - if (options.x == null) { - options.x = options.left; - } - } - if (options.top) { - if (options.y == null) { - options.y = options.top; - } - } - if (options.title == null) { - options.title = frameName; - } - if (options.width == null) { - options.width = 800; - } - if (options.height == null) { - options.height = 600; - } - - for (const name of ints) { - if (options[name] != null) { - options[name] = parseInt(options[name], 10); - } - } - + const { options, additionalFeatures } = convertFeaturesString(features, frameName); const referrer = { url: '', policy: 'default' }; - internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures); + internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures, null); }); // Routed window.open messages with fully parsed options function internalWindowOpen (event, url, referrer, frameName, disposition, options, additionalFeatures, postData) { options = mergeBrowserWindowOptions(event.sender, options); - event.sender.emit('new-window', event, url, frameName, disposition, options, additionalFeatures, referrer); + const postBody = postData ? { + data: postData, + ...parseContentTypeFormat(postData) + } : null; + + event.sender.emit('new-window', event, url, frameName, disposition, options, additionalFeatures, referrer, postBody); const { newGuest } = event; if ((event.sender.getType() === 'webview' && event.sender.getLastWebPreferences().disablePopups) || event.defaultPrevented) { if (newGuest != null) { diff --git a/lib/common/parse-features-string.js b/lib/common/parse-features-string.js index 90c6f7ad140a5..6d62be13df748 100644 --- a/lib/common/parse-features-string.js +++ b/lib/common/parse-features-string.js @@ -3,7 +3,7 @@ // parses a feature string that has the format used in window.open() // - `features` input string // - `emit` function(key, value) - called for each parsed KV -module.exports = function parseFeaturesString (features, emit) { +function parseFeaturesString (features, emit) { features = `${features}`.trim(); // split the string by ',' features.split(/\s*,\s*/).forEach((feature) => { @@ -18,4 +18,68 @@ module.exports = function parseFeaturesString (features, emit) { // emit the parsed pair emit(key, value); }); +} + +function convertFeaturesString (features, frameName) { + const options = {}; + + const ints = ['x', 'y', 'width', 'height', 'minWidth', 'maxWidth', 'minHeight', 'maxHeight', 'zoomFactor']; + const webPreferences = ['zoomFactor', 'nodeIntegration', 'enableRemoteModule', 'preload', 'javascript', 'contextIsolation', 'webviewTag']; + + // Used to store additional features + const additionalFeatures = []; + + // Parse the features + parseFeaturesString(features, function (key, value) { + if (value === undefined) { + additionalFeatures.push(key); + } else { + // Don't allow webPreferences to be set since it must be an object + // that cannot be directly overridden + if (key === 'webPreferences') return; + + if (webPreferences.includes(key)) { + if (options.webPreferences == null) { + options.webPreferences = {}; + } + options.webPreferences[key] = value; + } else { + options[key] = value; + } + } + }); + + if (options.left) { + if (options.x == null) { + options.x = options.left; + } + } + if (options.top) { + if (options.y == null) { + options.y = options.top; + } + } + if (options.title == null) { + options.title = frameName; + } + if (options.width == null) { + options.width = 800; + } + if (options.height == null) { + options.height = 600; + } + + for (const name of ints) { + if (options[name] != null) { + options[name] = parseInt(options[name], 10); + } + } + + return { + options, additionalFeatures + }; +} + +module.exports = { + parseFeaturesString, convertFeaturesString }; diff --git a/package.json b/package.json index 05cdd2a318ad8..75e4d84f14d18 100644 --- a/package.json +++ b/package.json @@ -142,4 +142,4 @@ "@types/multiparty": "^0.0.32", "@types/temp": "^0.8.34" } -} \ No newline at end of file +} diff --git a/patches/chromium/can_create_window.patch b/patches/chromium/can_create_window.patch index c6a269a2ad175..8ddbc7f169981 100644 --- a/patches/chromium/can_create_window.patch +++ b/patches/chromium/can_create_window.patch @@ -16,24 +16,41 @@ index 47749c430a71c3060c3e258ce1671bf6b03fa8c8..f2aedf5321a13742c6e3c2c4b7492b23 last_committed_origin_, params->window_container_type, params->target_url, params->referrer.To(), params->frame_name, params->disposition, *params->features, -+ params->additional_features, params->body, ++ params->raw_features, params->body, effective_transient_activation_state, params->opener_suppressed, &no_javascript_access); - + +diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc +index 4311fd59dfd8db1dfbebd898c43c3b48caf6cdd7..2ddb86702558920cb372d2c8b3423b2d53d68e56 100644 +--- a/content/browser/web_contents/web_contents_impl.cc ++++ b/content/browser/web_contents/web_contents_impl.cc +@@ -2910,9 +2910,9 @@ void WebContentsImpl::CreateNewWindow( + } + + if (delegate_) { +- delegate_->WebContentsCreated(this, render_process_id, +- opener->GetRoutingID(), params.frame_name, +- params.target_url, new_contents_impl); ++ delegate_->WebContentsCreatedWithFullParams(this, render_process_id, ++ opener->GetRoutingID(), ++ params, new_contents_impl); + } + + for (auto& observer : observers_) { diff --git a/content/common/frame.mojom b/content/common/frame.mojom index 00d9c34304266b49e3f9703ef1aea6524d026ed5..44bb5868b8c3699230d74de0f0903af572d000fc 100644 --- a/content/common/frame.mojom +++ b/content/common/frame.mojom @@ -303,6 +303,10 @@ struct CreateNewWindowParams { - + // The window features to use for the new window. blink.mojom.WindowFeatures features; + + // Extra fields added by Electron. -+ array additional_features; ++ string raw_features; + network.mojom.URLRequestBody? body; }; - + // Operation result when the renderer asks the browser to create a new window. diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc index 702cd0792df47809d9278dd3af40e0e70494e229..75cd0ad7a8dc1efd89683d8da192050659c5016f 100644 @@ -43,7 +60,7 @@ index 702cd0792df47809d9278dd3af40e0e70494e229..75cd0ad7a8dc1efd89683d8da1920506 const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, -+ const std::vector& additional_features, ++ const std::string& raw_features, + const scoped_refptr& body, bool user_gesture, bool opener_suppressed, @@ -58,17 +75,65 @@ index ed0460f7e3c9e2149eed89f7d35a4bb457f6b29f..5fc3d0e31927b618d04024fcc42b2061 struct ResourceRequest; +class ResourceRequestBody; } // namespace network - + namespace rappor { @@ -825,6 +826,8 @@ class CONTENT_EXPORT ContentBrowserClient { const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, -+ const std::vector& additional_features, ++ const std::string& raw_features, + const scoped_refptr& body, bool user_gesture, bool opener_suppressed, bool* no_javascript_access); +diff --git a/content/public/browser/web_contents_delegate.cc b/content/public/browser/web_contents_delegate.cc +index 5aa89aef4dbf3fe4ac7ce56b99c371d188584351..72a434fa612cdb4521aab981f98883af87783a80 100644 +--- a/content/public/browser/web_contents_delegate.cc ++++ b/content/public/browser/web_contents_delegate.cc +@@ -26,6 +26,17 @@ namespace content { + + WebContentsDelegate::WebContentsDelegate() = default; + ++void WebContentsDelegate::WebContentsCreatedWithFullParams( ++ WebContents* source_contents, ++ int opener_render_process_id, ++ int opener_render_frame_id, ++ const mojom::CreateNewWindowParams& params, ++ WebContents* new_contents) { ++ WebContentsCreated(source_contents, opener_render_process_id, ++ opener_render_frame_id, params.frame_name, ++ params.target_url, new_contents); ++} ++ + WebContents* WebContentsDelegate::OpenURLFromTab(WebContents* source, + const OpenURLParams& params) { + return nullptr; +diff --git a/content/public/browser/web_contents_delegate.h b/content/public/browser/web_contents_delegate.h +index 01db3853cb915dca4873c779f06bbf84b002abf6..4a8e4cf2386594e27e2dd8117bac78531b28830c 100644 +--- a/content/public/browser/web_contents_delegate.h ++++ b/content/public/browser/web_contents_delegate.h +@@ -16,6 +16,7 @@ + #include "base/strings/string16.h" + #include "build/build_config.h" + #include "content/common/content_export.h" ++#include "content/common/frame.mojom.h" + #include "content/public/browser/bluetooth_chooser.h" + #include "content/public/browser/bluetooth_scanning_prompt.h" + #include "content/public/browser/invalidate_type.h" +@@ -333,6 +334,13 @@ class CONTENT_EXPORT WebContentsDelegate { + const std::string& partition_id, + SessionStorageNamespace* session_storage_namespace); + ++ virtual void WebContentsCreatedWithFullParams( ++ WebContents* source_contents, ++ int opener_render_process_id, ++ int opener_render_frame_id, ++ const mojom::CreateNewWindowParams& params, ++ WebContents* new_contents); ++ + // Notifies the delegate about the creation of a new WebContents. This + // typically happens when popups are created. + virtual void WebContentsCreated(WebContents* source_contents, diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc index b3388258feb1d9fd791a975b08a382addd06af3a..98855151d1f07177a20b0f6079f2f48826d215ca 100644 --- a/content/renderer/render_view_impl.cc @@ -81,10 +146,12 @@ index b3388258feb1d9fd791a975b08a382addd06af3a..98855151d1f07177a20b0f6079f2f488 #include "content/renderer/media/audio/audio_device_factory.h" #include "content/renderer/render_frame_impl.h" #include "content/renderer/render_frame_proxy.h" -@@ -1255,6 +1256,8 @@ WebView* RenderViewImpl::CreateView( +@@ -1255,6 +1256,10 @@ WebView* RenderViewImpl::CreateView( } params->features = ConvertWebWindowFeaturesToMojoWindowFeatures(features); - + ++ params->raw_features = features.raw_features.Utf8( ++ WTF::UTF8ConversionMode::kStrictUTF8ConversionReplacingUnpairedSurrogatesWithFFFD); + params->body = GetRequestBodyForWebURLRequest(request); + // We preserve this information before sending the message since |params| is @@ -98,7 +165,7 @@ index bf7c4bf0b76a89b1ecee6edc11fcfd06fe679968..79b2ba79653a251d3a4c3a54adbcd5c1 const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, -+ const std::vector& additional_features, ++ const std::string& raw_features, + const scoped_refptr& body, bool user_gesture, bool opener_suppressed, @@ -111,8 +178,42 @@ index aaf3b8fed16bc2959941effaffd3aec87017ca59..1ea96df464f61c963a6d9aa224b9607e const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, -+ const std::vector& additional_features, ++ const std::string& raw_features, + const scoped_refptr& body, bool user_gesture, bool opener_suppressed, bool* no_javascript_access) override; +diff --git a/third_party/blink/public/web/web_window_features.h b/third_party/blink/public/web/web_window_features.h +index 4f735ad0d97eaac9a57dad137e479f8a7ec33a36..0a3c5821962c85609b64b3625fa6b8d658cd9ab2 100644 +--- a/third_party/blink/public/web/web_window_features.h ++++ b/third_party/blink/public/web/web_window_features.h +@@ -31,6 +31,8 @@ + #ifndef THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_WINDOW_FEATURES_H_ + #define THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_WINDOW_FEATURES_H_ + ++#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" ++ + namespace blink { + + struct WebWindowFeatures { +@@ -60,6 +62,8 @@ struct WebWindowFeatures { + bool noreferrer = false; + bool background = false; + bool persistent = false; ++ ++ String raw_features; + }; + + } // namespace blink +diff --git a/third_party/blink/renderer/core/frame/local_dom_window.cc b/third_party/blink/renderer/core/frame/local_dom_window.cc +index 07cb6e3fabfa7a32f9923dcf4fbc8cebccdbde46..58e768bfcac2561f4427c71b6c31ba73f9be4b7b 100644 +--- a/third_party/blink/renderer/core/frame/local_dom_window.cc ++++ b/third_party/blink/renderer/core/frame/local_dom_window.cc +@@ -1492,6 +1492,7 @@ DOMWindow* LocalDOMWindow::open(v8::Isolate* isolate, + } + + WebWindowFeatures window_features = GetWindowFeaturesFromString(features); ++ window_features.raw_features = features; + + FrameLoadRequest frame_request(active_document, + ResourceRequest(completed_url)); diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 9f280a889f1d2..95c1f81ef7e64 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -671,7 +671,7 @@ bool App::CanCreateWindow( const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, - const std::vector& additional_features, + const std::string& raw_features, const scoped_refptr& body, bool user_gesture, bool opener_suppressed, @@ -685,7 +685,7 @@ bool App::CanCreateWindow( // No need to emit any event if the WebContents is not available in JS. if (!api_web_contents.IsEmpty()) { api_web_contents->OnCreateWindow(target_url, referrer, frame_name, - disposition, additional_features, body); + disposition, raw_features, body); } } diff --git a/shell/browser/api/electron_api_app.h b/shell/browser/api/electron_api_app.h index 436d47f81739a..6ca93737e263c 100644 --- a/shell/browser/api/electron_api_app.h +++ b/shell/browser/api/electron_api_app.h @@ -131,7 +131,7 @@ class App : public ElectronBrowserClient::Delegate, const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, - const std::vector& additional_features, + const std::string& raw_features, const scoped_refptr& body, bool user_gesture, bool opener_suppressed, diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index adda4b6f812ed..5bd2e3be4d06a 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -44,6 +44,7 @@ #include "content/public/browser/site_instance.h" #include "content/public/browser/storage_partition.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/referrer_type_converters.h" #include "electron/buildflags/buildflags.h" #include "electron/shell/common/api/api.mojom.h" #include "gin/data_object_builder.h" @@ -641,25 +642,25 @@ void WebContents::OnCreateWindow( const content::Referrer& referrer, const std::string& frame_name, WindowOpenDisposition disposition, - const std::vector& features, + const std::string& features, const scoped_refptr& body) { - if (type_ == Type::BROWSER_WINDOW || type_ == Type::OFF_SCREEN) - Emit("-new-window", target_url, frame_name, disposition, features, body, - referrer); - else - Emit("new-window", target_url, frame_name, disposition, features); + Emit("-new-window", target_url, frame_name, disposition, features, referrer, + body); } -void WebContents::WebContentsCreated(content::WebContents* source_contents, - int opener_render_process_id, - int opener_render_frame_id, - const std::string& frame_name, - const GURL& target_url, - content::WebContents* new_contents) { +void WebContents::WebContentsCreatedWithFullParams( + content::WebContents* source_contents, + int opener_render_process_id, + int opener_render_frame_id, + const content::mojom::CreateNewWindowParams& params, + content::WebContents* new_contents) { ChildWebContentsTracker::CreateForWebContents(new_contents); auto* tracker = ChildWebContentsTracker::FromWebContents(new_contents); - tracker->url = target_url; - tracker->frame_name = frame_name; + tracker->url = params.target_url; + tracker->frame_name = params.frame_name; + tracker->referrer = params.referrer.To(); + tracker->raw_features = params.raw_features; + tracker->body = params.body; } void WebContents::AddNewContents( @@ -678,7 +679,8 @@ void WebContents::AddNewContents( CreateAndTake(isolate(), std::move(new_contents), Type::BROWSER_WINDOW); if (Emit("-add-new-contents", api_web_contents, disposition, user_gesture, initial_rect.x(), initial_rect.y(), initial_rect.width(), - initial_rect.height(), tracker->url, tracker->frame_name)) { + initial_rect.height(), tracker->url, tracker->frame_name, + tracker->referrer, tracker->raw_features, tracker->body)) { // TODO(zcbenz): Can we make this sync? api_web_contents->DestroyWebContents(true /* async */); } @@ -688,10 +690,8 @@ content::WebContents* WebContents::OpenURLFromTab( content::WebContents* source, const content::OpenURLParams& params) { if (params.disposition != WindowOpenDisposition::CURRENT_TAB) { - if (type_ == Type::BROWSER_WINDOW || type_ == Type::OFF_SCREEN) - Emit("-new-window", params.url, "", params.disposition); - else - Emit("new-window", params.url, "", params.disposition); + Emit("-new-window", params.url, "", params.disposition, "", params.referrer, + params.post_data); return nullptr; } diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 7f32581787dff..c13f59e406226 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -13,6 +13,7 @@ #include "base/observer_list.h" #include "base/observer_list_types.h" #include "content/common/cursors/webcursor.h" +#include "content/common/frame.mojom.h" #include "content/public/browser/devtools_agent_host.h" #include "content/public/browser/keyboard_event_processing_result.h" #include "content/public/browser/render_widget_host.h" @@ -312,7 +313,7 @@ class WebContents : public gin_helper::TrackableObject, const content::Referrer& referrer, const std::string& frame_name, WindowOpenDisposition disposition, - const std::vector& features, + const std::string& features, const scoped_refptr& body); // Returns the preload script path of current WebContents. @@ -383,12 +384,12 @@ class WebContents : public gin_helper::TrackableObject, const base::string16& message, int32_t line_no, const base::string16& source_id) override; - void WebContentsCreated(content::WebContents* source_contents, - int opener_render_process_id, - int opener_render_frame_id, - const std::string& frame_name, - const GURL& target_url, - content::WebContents* new_contents) override; + void WebContentsCreatedWithFullParams( + content::WebContents* source_contents, + int opener_render_process_id, + int opener_render_frame_id, + const content::mojom::CreateNewWindowParams& params, + content::WebContents* new_contents) override; void AddNewContents(content::WebContents* source, std::unique_ptr new_contents, WindowOpenDisposition disposition, diff --git a/shell/browser/child_web_contents_tracker.cc b/shell/browser/child_web_contents_tracker.cc index 9d1d0962ed516..51cfb8ba9cd2c 100644 --- a/shell/browser/child_web_contents_tracker.cc +++ b/shell/browser/child_web_contents_tracker.cc @@ -9,6 +9,8 @@ namespace electron { ChildWebContentsTracker::ChildWebContentsTracker( content::WebContents* web_contents) {} +ChildWebContentsTracker::~ChildWebContentsTracker() {} + WEB_CONTENTS_USER_DATA_KEY_IMPL(ChildWebContentsTracker) } // namespace electron diff --git a/shell/browser/child_web_contents_tracker.h b/shell/browser/child_web_contents_tracker.h index 985d03a6b51d7..a7cd335af864e 100644 --- a/shell/browser/child_web_contents_tracker.h +++ b/shell/browser/child_web_contents_tracker.h @@ -15,8 +15,13 @@ namespace electron { // created by native `window.open()` struct ChildWebContentsTracker : public content::WebContentsUserData { + ~ChildWebContentsTracker() override; + GURL url; std::string frame_name; + content::Referrer referrer; + std::string raw_features; + scoped_refptr body; private: explicit ChildWebContentsTracker(content::WebContents* web_contents); diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc index 784ea47619f7d..ace78c56861f1 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -762,7 +762,7 @@ bool ElectronBrowserClient::CanCreateWindow( const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, - const std::vector& additional_features, + const std::string& raw_features, const scoped_refptr& body, bool user_gesture, bool opener_suppressed, @@ -786,7 +786,7 @@ bool ElectronBrowserClient::CanCreateWindow( return delegate_->CanCreateWindow( opener, opener_url, opener_top_level_frame_url, source_origin, container_type, target_url, referrer, frame_name, disposition, features, - additional_features, body, user_gesture, opener_suppressed, + raw_features, body, user_gesture, opener_suppressed, no_javascript_access); } diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h index 658cee355ce75..1ac828b3230d9 100644 --- a/shell/browser/electron_browser_client.h +++ b/shell/browser/electron_browser_client.h @@ -128,7 +128,7 @@ class ElectronBrowserClient : public content::ContentBrowserClient, const std::string& frame_name, WindowOpenDisposition disposition, const blink::mojom::WindowFeatures& features, - const std::vector& additional_features, + const std::string& raw_features, const scoped_refptr& body, bool user_gesture, bool opener_suppressed, diff --git a/spec-main/internal-spec.ts b/spec-main/internal-spec.ts index f91fc5dda1077..a365eff309263 100644 --- a/spec-main/internal-spec.ts +++ b/spec-main/internal-spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; describe('feature-string parsing', () => { it('is indifferent to whitespace around keys and values', () => { - const parseFeaturesString = require('../lib/common/parse-features-string'); + const { parseFeaturesString } = require('../lib/common/parse-features-string'); const checkParse = (string: string, parsed: Record) => { const features: Record = {}; parseFeaturesString(string, (k: string, v: any) => { features[k] = v; });