feat: remove nativeWindowOpen option by nornagon · Pull Request #29405 · electron/electron

nornagon · 2021-05-28T19:04:15Z

Description of Change

nativeWindowOpen: true was made the default in 15.x.y. Let's remove the option to set it to false.

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation is changed or added
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Removed the old BrowserWindowProxy-based implementation of window.open. This also removes the nativeWindowOpen option from webPreferences.

nornagon · 2021-06-01T22:14:14Z

Blocked on #29463

miniak · 2021-06-02T01:28:00Z

@nornagon you can also remove openerId from InternalWebPreferences in typings/internal-ambient.d.ts

zcbenz

The mac/linux CI are crashing:

ok 35 chromium feature navigator.geolocation returns position when permission is granted


#
# Fatal error in ../../v8/src/api/api-inl.h, line 183
# Debug check failed: microtask_queue->GetMicrotasksScopeDepth() || !microtask_queue->DebugMicrotasksScopeDepthIsZero().
#
#
#
#FailureMessage Object: 0x7ffee33942500   Electron Framework                  0x0000000112010a79 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x0000000111f2c103 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x00000001143aa24d gin::(anonymous namespace)::PrintStackTrace() + 45
3   Electron Framework                  0x0000000113d80d16 V8_Fatal(char const*, int, char const*, ...) + 326
4   Electron Framework                  0x0000000113d806f5 v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) + 21
5   Electron Framework                  0x000000010f23e9e1 v8::CallDepthScope<true>::~CallDepthScope() + 865
6   Electron Framework                  0x000000010f1fbd53 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 835
7   Electron Framework                  0x00000001173b9d62 node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 738
8   Electron Framework                  0x00000001173ba02b node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 187
9   Electron Framework                  0x0000000117403175 node::Environment::CheckImmediate(uv_check_s*) + 181
10  Electron Framework                  0x000000010d7f352d uv__run_check + 157
11  Electron Framework                  0x000000010d7ed461 uv_run + 369
12  Electron Framework                  0x000000010d9b30bc electron::NodeBindings::UvRunOnce() + 348
13  Electron Framework                  0x000000010d9b4874 base::internal::Invoker<base::internal::BindState<void (electron::NodeBindings::*)(), base::WeakPtr<electron::NodeBindings> >, void ()>::RunOnce(base::internal::BindStateBase*) + 148
14  Electron Framework                  0x0000000111fa8050 base::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 400
15  Electron Framework                  0x0000000111fcc152 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow*) + 994
16  Electron Framework                  0x0000000111fcb978 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 88
17  Electron Framework                  0x00000001120270b1 base::MessagePumpCFRunLoopBase::RunWork() + 65
18  Electron Framework                  0x0000000112020862 base::mac::CallWithEHFrame(void () block_pointer) + 10
19  Electron Framework                  0x0000000112026adf base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
20  CoreFoundation                      0x00007fff3384e8f4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
21  CoreFoundation                      0x00007fff3384e893 __CFRunLoopDoSource0 + 103
22  CoreFoundation                      0x00007fff3384e6ad __CFRunLoopDoSources0 + 209
23  CoreFoundation                      0x00007fff3384d3c9 __CFRunLoopRun + 937
24  CoreFoundation                      0x00007fff3384c9c3 CFRunLoopRunSpecific + 466
25  Foundation                          0x00007fff35f081c8 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 212
26  Electron Framework                  0x00000001120276ee base::MessagePumpNSRunLoop::DoRun(base::MessagePump::Delegate*) + 126
27  Electron Framework                  0x000000011202648c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 140
28  Electron Framework                  0x0000000111fccd1b base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 651
29  Electron Framework                  0x0000000111f80c2a base::RunLoop::Run(base::Location const&) + 890
30  Electron Framework                  0x0000000117178efb content::RendererMain(content::MainFunctionParams const&) + 1611
31  Electron Framework                  0x000000010f17c6f7 content::ContentMainRunnerImpl::Run(bool) + 503
32  Electron Framework                  0x000000010f17b602 content::RunContentProcess(content::ContentMainParams const&, content::ContentMainRunner*) + 2658
33  Electron Framework                  0x000000010f17b6ec content::ContentMain(content::ContentMainParams const&) + 44
34  Electron Framework                  0x000000010d7fff17 ElectronMain + 135
35  Electron Helper (Renderer)          0x000000010c868eda main + 298
36  libdyld.dylib                       0x00007fff6d943cc9 start + 1
webContents 1 crashed: file:///Users/distiller/project/src/electron/spec/static/index.html?grep=&invert=&files=spec%2Fapi-clipboard-spec.js%2Cspec%2Fapi-process-spec.js%2Cspec%2Fapi-web-frame-spec.js%2Cspec%2Fchromium-spec.js%2Cspec%2Fwebview-spec.js (killed=false)
Renderer process crashed
Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
webContents 2 crashed:  (killed=false)
Renderer process crashed - see https://www.electronjs.org/docs/tutorial/application-debugging for potential debugging information.
[2832:0602/173246.272595:WARNING:discardable_shared_memory_manager.cc(432)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[2832:0602/173246.276213:ERROR:process_posix.cc(330)] Unable to terminate process 2835: No such process (3)
✗ Electron tests failed with code 1.

MarshallOfSound

nativeWindowOpen being made the default doesn't allow us to just remove it. It needs to go through a deprecation cycle just like everything else.

As far as I can see the Electron 14 PR to enable nativeWindowOpen by default also failed to go through a proper deprecation cycle #28552

The deprecation cycle for a default change should be as follows:

Version N - Initial state
Version N + 1 - Deprecation warning about initial state changing
Version N + 2 - Change initial state
Version N + 3 - Add deprecation warning that the flag will be removed
Version N + 4 - Remove the flag

We skipped over step N + 1 and N + 3 Electron 14 hasn't really existed for long though, so we can still fix this.

A good example of a deprecation timeline that worked really well IMO is the context aware module deprecation --> #18397

See #29463

nornagon · 2021-07-26T21:28:45Z

The deprecation cycle for a default change should be as follows:

Version N - Initial state

Version N + 1 - Deprecation warning about initial state changing

Version N + 2 - Change initial state

Version N + 3 - Add deprecation warning that the flag will be removed

Version N + 4 - Remove the flag

We skipped over step N + 1 and N + 3 Electron 14 hasn't really existed for long though, so we can still fix this.

We chose N=13. So:

14 - leaving nativeWindowOpen unspecified warns that the default will change.
15 - nativeWindowOpen: true becomes the default.
16 - specifying nativeWindowOpen: false will result in a deprecation warning.
17 - the nativeWindowOpen option will be removed.

Should we consider lengthening this timeline on account of the new release cadence? Perhaps we should target the final removal for 18 instead of 17?

miniak · 2021-11-17T00:28:21Z

@zcbenz I am still seeing those crashes when running tests, I will probably need some help with that if possible.

zcbenz · 2021-11-17T02:37:34Z

The node::FreeEnvironment implicitly executes callbacks, adding a microtasks scope fixes the crash.

miniak · 2021-11-22T01:42:08Z

@zcbenz there is still one test failing:

not ok 39 chromium feature window.open accepts "nodeIntegration" as feature
  AssertionError: expected false to be true
      at Context.<anonymous> (/Users/distiller/project/src/electron/spec/chromium-spec.js:71:61)

It's caused by this bug #31949

Deprecation cycle now respected

docs/breaking-changes.md

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

miniak · 2022-01-06T15:41:10Z

@nornagon #32116 is merged, this should be finally ready for merging

miniak · 2022-01-06T16:44:53Z

API LGTM

nornagon · 2022-01-06T17:08:19Z

API LGTM

release-clerk · 2022-01-06T17:28:06Z

Release Notes Persisted

Removed the old BrowserWindowProxy-based implementation of window.open. This also removes the nativeWindowOpen option from webPreferences.

Co-authored-by: Cheng Zhao <zcbenz@gmail.com> Co-authored-by: Milan Burda <milan.burda@gmail.com>

electron/electron#29405

carlaarchuleta308-source

gh pr checkout 29405

feat: remove nativeWindowOpen option

c9444d0

nornagon requested review from a team as code owners May 28, 2021 19:04

nornagon added the semver/major incompatible API changes label May 28, 2021

electron-cation bot added new-pr 🌱 PR opened recently api-review/requested 🗳 labels May 28, 2021

nornagon added the no-backport label May 28, 2021

patchup bot and others added 3 commits May 28, 2021 19:14

chore: update patches

1918695

lint

ae6c517

remove unused ipcs

73661b0

miniak approved these changes Jun 2, 2021

View reviewed changes

remove more references to openerId

672cd27

zcbenz previously requested changes Jun 3, 2021

View reviewed changes

MarshallOfSound previously requested changes Jun 3, 2021

View reviewed changes

electron-cation bot removed the new-pr 🌱 PR opened recently label Jun 4, 2021

nornagon added 3 commits July 26, 2021 14:39

Merge remote-tracking branch 'origin/main' into remove-nativewindowopen

d37d5e9

Merge remote-tracking branch 'origin/main' into remove-nativewindowopen

45ac15d

fix patch

16be850

miniak force-pushed the remove-nativewindowopen branch 5 times, most recently from 6984077 to bc2a39b Compare November 16, 2021 20:52

nornagon and others added 3 commits January 3, 2022 16:50

Merge remote-tracking branch 'origin/main' into remove-nativewindowopen

5c15ec9

feat(miniak): remove nativeWindowOpen option

279e737

chore: update comments about nativeWindowOpen

4ab743f

nornagon force-pushed the remove-nativewindowopen branch from e15e89e to 4ab743f Compare January 4, 2022 00:56

nornagon commented Jan 4, 2022

View reviewed changes

docs/breaking-changes.md Outdated Show resolved Hide resolved

Update docs/breaking-changes.md

06e3d18

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

VerteDinde approved these changes Jan 6, 2022

View reviewed changes

nornagon merged commit d44a187 into main Jan 6, 2022

nornagon deleted the remove-nativewindowopen branch January 6, 2022 17:28

t57ser pushed a commit to t57ser/electron that referenced this pull request Jan 25, 2022

feat: remove nativeWindowOpen option (electron#29405)

2235661

Co-authored-by: Cheng Zhao <zcbenz@gmail.com> Co-authored-by: Milan Burda <milan.burda@gmail.com>

commit-lint bot mentioned this pull request Mar 21, 2022

[pull] main from electron:main Blaquez-home/electron#3

Open

BitYoungjae added a commit to bgpworks/boxhero-electron that referenced this pull request May 3, 2022

더이상 유효하지 않는 nativeWindowOpen 옵션 제거

1d17f3d

electron/electron#29405

BitYoungjae mentioned this pull request May 3, 2022

chore: target=_blank인 a 태그로 윈도우 오픈시 외부 브라우저로 열리도록 bgpworks/boxhero-electron#55

Closed

miniak mentioned this pull request Jun 10, 2022

chore: remove unused proxy-window-open.snapshot.txt #34501

Merged

2 tasks

This was referenced Oct 9, 2022

Remove deprecated nativeWindowOpen option from webPreferences in help module manusa/electronim#152

Closed

Remove deprecated nativeWindowOpen option from webPreferences in chrome-tabs module manusa/electronim#155

Closed

This was referenced Nov 4, 2022

Upgrade to Electron 21 LedgerHQ/ledger-live#1463

Closed

Fix webview usage with electron 21 LedgerHQ/ledger-live#1777

Merged

Keith-CY mentioned this pull request Dec 6, 2022

chore(deps-dev): bump electron from 16.2.8 to 18.3.7 nervosnetwork/neuron#2525

Closed

This was referenced Dec 22, 2022

fix: webview usage with electron 22 LedgerHQ/ledger-live#2181

Closed

fix: webview usage with electron 22 LedgerHQ/ledger-live#2182

Merged

ErikSin mentioned this pull request Jan 26, 2023

Upgrage Electron and Bundler digidem/mapeo-desktop#715

Closed

3 tasks

miniak mentioned this pull request Jun 19, 2023

chore: remove deleted guest-window-proxy.ts from CODEOWNERS #38848

Merged

carlaarchuleta308-source reviewed Feb 18, 2026

View reviewed changes

MarshallOfSound mentioned this pull request Mar 25, 2026

refactor: remove dead named-window lookup from guest-window-manager #50474

Merged

Conversation

nornagon commented May 28, 2021 • edited by miniak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

nornagon commented Jun 1, 2021

Uh oh!

miniak commented Jun 2, 2021

Uh oh!

zcbenz left a comment

Choose a reason for hiding this comment

Uh oh!

MarshallOfSound left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nornagon commented Jul 26, 2021

Uh oh!

miniak commented Nov 17, 2021

Uh oh!

zcbenz commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miniak commented Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

miniak commented Jan 6, 2022

Uh oh!

miniak commented Jan 6, 2022

Uh oh!

nornagon commented Jan 6, 2022

Uh oh!

release-clerk bot commented Jan 6, 2022

Uh oh!

carlaarchuleta308-source left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nornagon commented May 28, 2021 •

edited by miniak

Loading

MarshallOfSound left a comment •

edited

Loading

zcbenz commented Nov 17, 2021 •

edited

Loading

miniak commented Nov 22, 2021 •

edited

Loading