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 "Open destination folder" on macOS #17305

Merged
merged 2 commits into from
Jul 4, 2022
Merged

Fix "Open destination folder" on macOS #17305

merged 2 commits into from
Jul 4, 2022

Conversation

Kolcha
Copy link
Contributor

@Kolcha Kolcha commented Jul 1, 2022

actually 2 issues where fixed (see commit messages for details):

  • the issue caused crash (the second commit)
  • incorrect path was passed to openFiles()

Torrent::contentPath() already returns correct abolute path to the torrent content, it considers any possible situations

existing code tried to append some "prefix" and that lead to not existing "slightly duplicated" path like

"/Users/nickkorotysh/Downloads/Users/nickkorotysh/Downloads/ubuntu-22.04-desktop-amd64.iso"

so destination folder was not opened in any case (but no crash happens in such case)

crash was not actually fixed, its source somewhere deep in Qt, just some workaround added, see corresponding comment in the code.

build was tested on both MacBooks: Intel-based (Core i9, 2019) and M1-based (M1 Pro, 2021)

Fixes #17273

it already provides absolute path, no additional manipulations
are required.
@Chocobo1 Chocobo1 added the OS: macOS Issues specific to macOS label Jul 2, 2022
@Chocobo1 Chocobo1 added this to the 4.5.0 milestone Jul 2, 2022
Chocobo1
Chocobo1 previously approved these changes Jul 2, 2022
src/gui/transferlistwidget.cpp Show resolved Hide resolved
src/gui/macutilities.mm Outdated Show resolved Hide resolved
src/gui/macutilities.mm Outdated Show resolved Hide resolved
@glassez glassez changed the title Fixed "Open destination folder" on macOS Fix "Open destination folder" on macOS Jul 2, 2022
@glassez
Copy link
Member

glassez commented Jul 2, 2022

Torrent::contentPath() already returns correct absolute path

Thank you for the fix! macOS related code, apparently, has not been changed/fixed due to the fact that it remains behind the scenes when developing on other platforms.

In some unknown way, the one line in Objective-C affects Qt's main
loop causing the crash in QApplication::exec() on processing next
event after that call.

Even crash doesn't happen exactly after this call, it will happen
on application exit. Call stack and disassembly are the same in
all cases.

But running that code in another thread solves the issue.
@@ -106,7 +106,15 @@ void openFiles(const PathList &pathList)
for (const auto &path : pathList)
[pathURLs addObject:[NSURL fileURLWithPath:path.toString().toNSString()]];

[[NSWorkspace sharedWorkspace] activateFileViewerSelectingURLs:pathURLs];
Copy link
Member

Choose a reason for hiding this comment

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

Since this is a Qt related issue maybe try to solve it without touching macOS related code (which is still correct)? I mean something like invoking openFiles using queued invocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

initially I did. but openFiles() is used in multiple places, so decided to add workaround exactly inside that function

@Chocobo1 Chocobo1 merged commit ec1d2cb into qbittorrent:master Jul 4, 2022
@Chocobo1
Copy link
Member

Chocobo1 commented Jul 4, 2022

@Kolcha
Thank you!

Also this should be back port to v4.4.x

@Kolcha Kolcha deleted the fix/open-dest-dir branch July 4, 2022 06:21
@Chocobo1 Chocobo1 mentioned this pull request Jul 5, 2022
Chocobo1 pushed a commit to Chocobo1/qBittorrent that referenced this pull request Jul 5, 2022
In some unknown way, the one line in Objective-C affects Qt's main
loop causing the crash in QApplication::exec() on processing next
event after that call.

Even crash doesn't happen exactly after this call, it will happen
on application exit. Call stack and disassembly are the same in
all cases.

But running that code in another thread solves the issue.

Original PR: qbittorrent#17305.
Chocobo1 pushed a commit that referenced this pull request Jul 6, 2022
In some unknown way, the one line in Objective-C affects Qt's main
loop causing the crash in QApplication::exec() on processing next
event after that call.

Even crash doesn't happen exactly after this call, it will happen
on application exit. Call stack and disassembly are the same in
all cases.

But running that code in another thread solves the issue.

Original PR: #17305.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS: macOS Issues specific to macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Open destination folder" action quits.
3 participants