fixed 'copy key and link'

[PremKolar]

'copy key and link' from right-click menu creates a string that is not a url. it includes the url, but it also includes the key.
setHtmlContent() was not appropriate.

Fixes #5835
#5835

• Change in CHANGELOG.md described (if applicable)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Screenshots added in PR description (for UI changes)
• Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

[tobiasdiez]

Welcome to JabRef, and thanks a lot for your contribution.

The idea to use setContent is the correct one. However, the html content should also be preserved. How this can be done is outlined below.

[tobiasdiez]

This really puts the html string into the clipboard, so if you paste it into say notepad you get <a href="url">key</a>. Instead, its better to add the html, and in addition also a fallback string of the format key - url. In this way, you get the desired behavior also when you paste in applications that do understand html (e.g onenote).

In order for this to work, you probably need to add a second fallback argument to

jabref/src/main/java/org/jabref/gui/ClipBoardManager.java

Lines 140 to 145 in 3db4313

	public void setHtmlContent(String html) {
	final ClipboardContent content = new ClipboardContent();
	content.putHtml(html);
	clipboard.setContent(content);
	setPrimaryClipboardContent(content);
	}

which is then added to the ClipboardContent via putString.

[PremKolar]

Hi @tobiasdiez !
Ah ok that makes sense.. I guess I should have looked deeper into what setHtmlContent() actually does.. I assumed it would be applicable for url's on their own only. So that means, both, setContent() and setHtmlContent() should be called?
Sorry for my noobness! This was my first open-source contribution ever :D
Can I fix the issue tonight (German time) and commit the changes?
thanks,
Niko

[tobiasdiez]

No worries, you were on the right track. Calling setContent and setHtmlContent is the right idea, but I suspect that this will push two different clipboard items. Instead, the setHtmlContent should be modified as follows:

public void setHtmlContent(String html, String fallbackPlain) { 
     final ClipboardContent content = new ClipboardContent(); 
     content.putHtml(html);
     content.putString(fallbackPlain);
     clipboard.setContent(content); 
     setPrimaryClipboardContent(content); 
 }

This should add one clipboard item, containing both the html as well as the normal string. (I hope)

Let me know if you encounter any further questions. There is not such a thing as noobness. We all can learn more. And learning in a relaxed environment is one of the main points of open source, so don't worry. Happy coding!

[PremKolar]

but I suspect that this will push two different clipboard items.

That was exactly my next thought :)
Ok, I will try..
Thanks for the kind words!

[PremKolar]

alright, i hope i did what you meant. I have to say, I still don't understand why copyKeyAndLink() creates html content for the clipboard, but e.g. copyKeyAndTitle() does not?
cheers,
Niko

[koppor]

Please remove the comment

[tobiasdiez]

Can the old setHtmlContent method be removed? If it's still used by other code, it might be a good idea to provide a fallback string in these cases as well. What do you think?

[PremKolar]

Hi Tobias,
If I am not mistaken, there is no usage of setHtmlContent(String html) anywhere...
Only setHtmlContent(String html, String fallbackPlain) is used once on line 230 of CopyMoreAction.java.
Should I remove the unused version?

[koppor]

Yes, just remove the the unused method.

[koppor]

I think, there were issues with git. I still have some code comments.

[Siedlerchr]

Codewise looks good. Plase fix the remaining checkstyle errors. Just click on the Details of the failing tests to see them