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.

Sign up for GitHub

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

Jump to bottom

Clicking a DOI link in the preview pane no longer crashes #8255

Merged
merged 6 commits into from
Nov 20, 2021
Merged

Clicking a DOI link in the preview pane no longer crashes #8255

merged 6 commits into from
Nov 20, 2021

Conversation

evanpickett
Copy link
Contributor

@evanpickett evanpickett commented Nov 16, 2021 •
edited by Siedlerchr
Loading

Fixes #7706
DOI links in the preview pane no longer open in the WebView and no longer cause uncaught exceptions as a consequence. Unfortunately wasn't able to find a clean solution to opening a browser window on all OS's without using java.awt

  • Change in CHANGELOG.md described in a way that is understandable for the average user (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, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@evanpickett evanpickett changed the title Clicking a DOI link in the preview pane opens web browser Clicking a DOI link in the preview pane no longer crashes Nov 17, 2021
@calixtus
Copy link
Member

Hi, thanks for your PR. You can use JabRefDesktop::openBrowser to open a url instead of directly using awt-methods.
I think openBrowser uses awt in the end too, but it's at least a common method which behaviour can be changed eventually.

@evanpickett
Copy link
Contributor Author

Hi, thanks for your PR. You can use JabRefDesktop::openBrowser to open a url instead of directly using awt-methods. I think openBrowser uses awt in the end too, but it's at least a common method which behaviour can be changed eventually.

Got it, I went ahead and re-added that functionality then

@calixtus
Copy link
Member

Great, thanks. Looks good to me! Looking forward. To see more from you eventually!

calixtus
calixtus requested changes Nov 19, 2021
View reviewed changes
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Just overlooked an issue. Sorry. 😬

src/main/java/org/jabref/gui/preview/PreviewViewer.java Outdated Show resolved Hide resolved
@Siedlerchr
Copy link
Member

@evanpickett Would be cool if you could fix the little remaining issue, just use LOGGER.error() so we can merge this

@evanpickett
Copy link
Contributor Author

Sorry I didn't respond quicker, let me know if you need any more changes

Siedlerchr
Siedlerchr approved these changes Nov 20, 2021
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks for the fast follow up. Just wanted to get this in! 😉

@Siedlerchr Siedlerchr merged commit d0d3d5e into JabRef:main Nov 20, 2021
Siedlerchr added a commit that referenced this pull request Nov 22, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into searchColPrefs
e367b1e 
* upstream/main:
  Observable Preferences H (WorkingDir, getUser, NameFormatter, Version, SpecialFields) (#8260)
  Bump checkstyle from 9.0.1 to 9.1 (#8266)
  Bump mockito-core from 4.0.0 to 4.1.0 (#8267)
  Bump classgraph from 4.8.130 to 4.8.135 (#8268)
  Fix some fetcher tests (#8258)
  Clicking a DOI link in the preview pane no longer crashes (#8255)
@mlep
Copy link
Contributor

mlep commented Nov 25, 2021

Fixed in JabRef 5.4--2021-11-25--7aaf5e2
Thank you @evanpickett

@ThiloteE
Copy link
Member

ThiloteE commented Nov 26, 2021 •
edited
Loading

Edit. Forget what i said. All is fine.

Siedlerchr added a commit that referenced this pull request Nov 30, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into jabrefonline
498a8e9 
* upstream/main:
  Consider directory pattern when checking if a file can be moved (#8244)
  Bump byte-buddy-parent from 1.12.1 to 1.12.2 (#8285)
  Bump unirest-java from 3.13.3 to 3.13.4 (#8283)
  Bump checkstyle from 9.1 to 9.2 (#8284)
  Bump classgraph from 4.8.135 to 4.8.137 (#8282)
  Do not resize main table columns in search dialog window (#8253)
  Fix NegativeArraySizeException (#8270)
  Update deployment.yml
  TEst
  Grand unified library properties (GRULPS) (#8264)
  Fixes RFC fetcher test case (#8271)
  Update Gradle Wrapper from 7.2 to 7.3 (#8246)
  Observable Preferences H (WorkingDir, getUser, NameFormatter, Version, SpecialFields) (#8260)
  Bump checkstyle from 9.0.1 to 9.1 (#8266)
  Bump mockito-core from 4.0.0 to 4.1.0 (#8267)
  Bump classgraph from 4.8.130 to 4.8.135 (#8268)
  Fix some fetcher tests (#8258)
  Clicking a DOI link in the preview pane no longer crashes (#8255)
@Siedlerchr Siedlerchr mentioned this pull request Dec 2, 2021
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calixtus calixtus calixtus requested changes

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DOI link: opens in preview and causes an exception
5 participants
@evanpickett @calixtus @Siedlerchr @mlep @ThiloteE