fix: window.open not overriding parent's webPreferences#32057
Conversation
| // Do a delayed Node.js initialization for child window. | ||
| // Check DidInstallConditionalFeatures below for the background. | ||
| auto* web_frame = render_frame_->GetWebFrame(); | ||
| if (has_delayed_node_initialization_ && web_frame->Opener() && |
There was a problem hiding this comment.
When browser side navigation is triggered, won't there be another call to ElectronRenderFrameObserver::DidInstallConditionalFeatures from https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/bindings/core/v8/local_window_proxy.cc;l=216-220 which will then initialize based on the correct preferences, is this second redirection via ElectronRenderFrameObserver::DidClearWindowObject needed ?
There was a problem hiding this comment.
For child window opened by window.open, the DidInstallConditionalFeatures is only called once, and DidClearWindowObject is instead called multiple times.
It seems that Chromium immediately creates an empty DOM window when window.open is called, and then does an in-place initialization later when the actual navigation happens, it does not create a new DOM window for the navigation.
I think this behavior is kind of reasonable, since the window object returned by window.open is immediately accessible to users even before the actual navigation starts.
bc16577 to
7ed393a
Compare
|
I have updated the PR with another approach that does not need to patch Chromium. I also found a Chromium issue that has some information on how |
|
I'm also removing the "nativeWindowOpen: false" option from renderer tests, which was required because some tests fail for the native |
5d1439c to
5220d0b
Compare
|
Release Notes Persisted
|
|
I was unable to backport this PR to "13-x-y" cleanly; |
|
I was unable to backport this PR to "14-x-y" cleanly; |
|
I have automatically backported this PR to "17-x-y", please check out #32107 |
|
I have automatically backported this PR to "16-x-y", please check out #32108 |
* fix: window.open not overriding parent's webPreferences * test: remove "nativeWindowOpen: false" from renderer tests
…)" This reverts commit 35ac7fb.
Description of Change
This PR fixes an issue that, child window opened with
window.opencan not override its web preferences inherited from the parent window, close #31949, close #31959.Please check the comments in
electron_render_frame_observer.ccfor the background of the problem.Fix PR also fixes an issue introduced by #31031 that, the feature string in
window.openwas treated aswindowOpenOverriddenOptionswhich would override thesecurityWebPreferences. This problem was hidden by the above bug 31949 because the web preferences could not be overridden, but with the bug fixed 31031 would then allow features string to override security options.There are also changes to tests as some of them rely on the bug to work.
Release Notes
Notes: Fix
window.opennot overriding parent'swebPreferences.