Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'Object has been destroyed' when FreeTube is passed a URL with no windows open on macOS #6278

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

OothecaPickle
Copy link
Contributor

@OothecaPickle OothecaPickle commented Dec 4, 2024

Fix 'Object has been destroyed' when FreeTube is passed a URL with no windows open on macOS

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Relates to @srolandmarshall's comment on #4762 (but unrelated to the issue commented on).

Description

Because FreeTube does not quit automatically when all windows are closed on macOS (as on other plaforms), the main process tries to fire an IPC message to a non-existent mainWindow when it receives an open-url event or a command line URL argument. This results in an Object has been destroyed error message.

This PR sets mainWindow to null when all FreeTube windows have been closed, and, when this is the case, has the main process create a new window and pass it a URL to be opened on open-url (if app.isReady() is true) or second-instance (with a command line URL argument) events.

Screenshots

Testing

  1. Open FreeTube on macOS and close its window without quitting the application (the FreeTube icon should remain visible in the Dock).
  2. Pass FreeTube a video URL either as a command line argument or by opening a freetube:// link (deep link).
  3. Check that video opens in a new window without an error.

Desktop

  • OS: macOS
  • OS Version: 10.13.6
  • FreeTube version: 0e44e6b

Additional context

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 4, 2024 22:32
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 4, 2024
src/main/index.js Outdated Show resolved Hide resolved
src/main/index.js Outdated Show resolved Hide resolved
src/main/index.js Outdated Show resolved Hide resolved
src/main/index.js Outdated Show resolved Hide resolved
@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Dec 4, 2024
auto-merge was automatically disabled December 5, 2024 00:40

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 5, 2024 00:40
@OothecaPickle OothecaPickle requested a review from absidue December 5, 2024 00:47
@OothecaPickle OothecaPickle marked this pull request as draft December 5, 2024 00:48
auto-merge was automatically disabled December 5, 2024 00:48

Pull request was converted to draft

(trying to launch a second instance wouldn't have focused main window)
@OothecaPickle OothecaPickle marked this pull request as ready for review December 5, 2024 01:00
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 5, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 5, 2024 01:01
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 5, 2024
@ChunkyProgrammer ChunkyProgrammer added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Dec 5, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on macOS 13.7.1
Bug has been destroyed

@kommunarr
Copy link
Collaborator

Don't currently have access to a Mac to validate, but looks good from static analysis of the code.

@efb4f5ff-1298-471a-8973-3d47447115dc

@kommunarr will you have access to it anytime soon or should we go ahead and approve this PR as @PikachuEXE tested it on their machine?

@FreeTubeBot FreeTubeBot merged commit 4a60974 into FreeTubeApp:development Dec 9, 2024
11 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 9, 2024
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Dec 10, 2024
* development: (96 commits)
  Move Invidious API calls into Invidious file (FreeTubeApp#6313)
  Translated using Weblate (Spanish)
  Use word-break on live chat error message (FreeTubeApp#6327)
  Bump youtubei.js from 11.0.1 to 12.0.0 (FreeTubeApp#6323)
  Bump shaka-player from 4.12.3 to 4.12.4 (FreeTubeApp#6318)
  Make seekbar and player controls appear at bottom of video and take full width (FreeTubeApp#6007)
  Fix 'Object has been destroyed' when FreeTube is passed a URL with no windows open on macOS (FreeTubeApp#6278)
  Bump mikefarah/yq from 4.44.5 to 4.44.6 (FreeTubeApp#6324)
  Translated using Weblate (French)
  Bump sass-loader from 16.0.3 to 16.0.4 (FreeTubeApp#6319)
  Bump webpack from 5.96.1 to 5.97.1 (FreeTubeApp#6322)
  Bump sass from 1.81.0 to 1.82.0 (FreeTubeApp#6320)
  Bump lefthook from 1.8.5 to 1.9.0 (FreeTubeApp#6321)
  Bump @intlify/eslint-plugin-vue-i18n from 3.1.0 to 3.2.0 (FreeTubeApp#6317)
  Translated using Weblate (Estonian)
  Translated using Weblate (Basque)
  Translated using Weblate (Icelandic)
  Translated using Weblate (Hungarian)
  Translated using Weblate (German)
  Fix channel home page list view (FreeTubeApp#6316)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants