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: Removed GPL code from FilesLauncher #11509

Merged
merged 10 commits into from
Apr 3, 2023

Conversation

0x5bfa
Copy link
Member

@0x5bfa 0x5bfa commented Mar 1, 2023

Description

I found the codes licensed under the GPL v3, though this whole project is licensed MIT license.

Motivation and Context

GPL code cannot be existed in Files.

NOTE CONTRIBUTORS: Even if you want to revert changes, you definitely should not. Please find another way.

Tasks

  • Remove codes that reference GPL code.
  • Remove GPL code.

Validation

  • Built and ran the app
  • Tested the changes for accessibility

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 1, 2023

@yaira2 @gave92 How do you think of this?

Files.sln Outdated Show resolved Hide resolved
@yaira2 yaira2 changed the title Remove GPL codes in FilesLauncher Fix: Removed GPL code from FilesLauncher Mar 1, 2023
@0x5bfa 0x5bfa marked this pull request as ready for review March 1, 2023 11:49
@hishitetsu
Copy link
Member

hishitetsu commented Mar 1, 2023

Isn't there any regression to this change?

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 1, 2023

Oppps. Sorry for being irresponsible. We have to discuss this part. This was the problem code that reference to the GPL code and I commented out. I pretty sure that they can be removed safely...

src/Files.OpenDialog/FilesLauncher/OpenInFolder.cpp

HRESULT OpenInFolder::NotifyShellOfNavigation(PCIDLIST_ABSOLUTE pidl)
{

...

	RETURN_IF_FAILED(m_shellWindows->RegisterPending(GetCurrentThreadId(), &pidlVariant, &empty,
		SWC_BROWSER, &m_shellWindowCookie));

	auto document = winrt::make_self<DocumentServiceProvider>();
	auto shellView = winrt::make_self<ShellView>(this, pidl);
	document->RegisterService(IID_IFolderView, shellView.get());

	auto browserApp = winrt::make_self<WebBrowserApp>(m_hwnd, document.get());

	long registeredCookie;
#pragma warning(push)
#pragma warning(                                                                                   \
	disable : 4311 4302) // 'reinterpret_cast': pointer truncation from 'HWND' to 'long',
	// 'reinterpret_cast': truncation from 'HWND' to 'long'
	RETURN_IF_FAILED(m_shellWindows->Register(browserApp.get(), reinterpret_cast<long>(m_hwnd),
		SWC_BROWSER, &registeredCookie));
#pragma warning(pop)

...

	return S_OK;
}

@hishitetsu
Copy link
Member

I have tested it. It seems to work well except highlighting a file as noted here.
I think this level of regression would be acceptable.

@yaira2
Copy link
Member

yaira2 commented Mar 12, 2023

Can you fix the merge conflict?

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 18, 2023

Done here. requesting a new review.

@hishitetsu
Copy link
Member

@onein528 Could you merge the main branch and build FilesLauncher again? The file FilesLauncher.exe.sha256 should be generated.

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 18, 2023

I'll be able to work on that the next week

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 27, 2023

@hishitetsu What did you say before on Discord? I remember a bit you were talking about unneeded codes.

@hishitetsu
Copy link
Member

What did you say before on Discord? I remember a bit you were talking about unneeded codes.

About this.

@0x5bfa
Copy link
Member Author

0x5bfa commented Mar 28, 2023

How can I fix this conflict

Btw, Ready for review.

@hishitetsu
Copy link
Member

hishitetsu commented Mar 28, 2023

How can I fix this conflict

FilesystemOperationDialog.xaml is modified in #11557, so you can import the one in the main branch.

@hishitetsu
Copy link
Member

If you say about FilesLauncher.exe, you can import the one in your branch, merge the upstream, then build FilesLauncher again.

@0x5bfa
Copy link
Member Author

0x5bfa commented Apr 2, 2023

@hishitetsu Thank you:)

FYI
To avoid conflict, I reverted changes in the exe file and I'm going to open another PR to update the assembly after this merged.

Finally! @yaira2 Ready for review.

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Apr 3, 2023
@yaira2 yaira2 merged commit 1e35d65 into files-community:main Apr 3, 2023
@0x5bfa 0x5bfa deleted the 5bfa/renovate-launcher branch April 19, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants