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

issue(8448): allow URL with + sign in entry tabs #8508

Merged
merged 1 commit into from
Feb 26, 2022
Merged

issue(8448): allow URL with + sign in entry tabs #8508

merged 1 commit into from
Feb 26, 2022

Conversation

alexandre-cremieux
Copy link
Contributor

@alexandre-cremieux alexandre-cremieux commented Feb 19, 2022 •
edited
Loading

Fixes #8448

  • What: Allow the URL fields to accept URL containing + sign.
  • Why: Some URLs are changed by JabRef when copy/paste from a navigation bar, making a bib pointing to an false URL.
  • How: this is done by replacing all the + sign UTF-8 code by it's escape sequence code %2b. The code is placed in the CleanupUrlFormatter in the org.jabref.logic.formatter.bibtexfields package. A test with the given case is provided also.

Note: please let me know in the PR comments what type of comment you want in CHANGELOG.md and if you need a screenshot.

  • 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.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 19, 2022
@Siedlerchr
Copy link
Member

Siedlerchr commented Feb 19, 2022 •
edited
Loading

Looks good so far to me. Will give it a test later. For the changelog entry look at the others, the Changelog is directed towards end users so something like: We fixed an issue where a plus sign in the url field in the entry editor was breaking links...
Screenshot is only necessary if you did some visible changes to a dialog/to the UI etc (added/removed things, redesigned...)

@alexandre-cremieux alexandre-cremieux marked this pull request as ready for review February 19, 2022 13:54
Siedlerchr
Siedlerchr approved these changes Feb 19, 2022
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! Tested it locally and works fine!

@alexandre-cremieux
Copy link
Contributor Author

You are welcome. Happy to help.

koppor
koppor approved these changes Feb 20, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for working on this - and also adding a test case.

I have some code formatting comments; other than that: LGTM

src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated Show resolved Hide resolved
koppor
koppor requested changes Feb 20, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

The code formatting issues need to be resolved - I clicked the wrong button - sorry for that! 😇

@alexandre-cremieux
issue(8448): allow URL with + sign in entry tabs
cb2fd26 
* clean code
* add java doc
* test

issue: JabRef#8448
@alexandre-cremieux
Copy link
Contributor Author

If you can please tell me where I can find the localization script for my understanding ? Thanks

@Siedlerchr
Copy link
Member

You just have to run the LocalizationConsistencyTest. It will tell you if there are any errors or strings missing/obsolete
See also https://devdocs.jabref.org/getting-into-the-code/code-howtos#using-localization-correctly

@Siedlerchr Siedlerchr merged commit f17eb40 into JabRef:main Feb 26, 2022
Siedlerchr added a commit that referenced this pull request Feb 26, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-changed
e3feb6a 
* upstream/main:
  issue(8448): allow URL with + sign in entry tabs (#8508)
Siedlerchr added a commit that referenced this pull request Mar 7, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fixUrlLink
e21e236 
* upstream/main:
  Bump guava from 31.0.1-jre to 31.1-jre (#8543)
  Bump org.beryx.jlink from 2.24.4 to 2.25.0 (#8548)
  Bump postgresql from 42.3.2 to 42.3.3 (#8546)
  Bump richtextfx from 0.10.7 to 0.10.9 (#8547)
  Bump archunit-junit5-engine from 0.22.0 to 0.23.1 (#8545)
  Bump actions/checkout from 2 to 3 (#8542)
  Squashed 'buildres/csl/csl-styles/' changes from eb97405..8f69d4e
  Bump classgraph from 4.8.139 to 4.8.141 (#8535)
  Bump archunit-junit5-api from 0.22.0 to 0.23.1 (#8536)
  issue(8448): allow URL with + sign in entry tabs (#8508)
  Add initial telemetry development documentation (#8530)
  Add existing "Remote storage" to index
  Disable telemetry client due to incompatibilites with jakarta (#8526)
  Update to jakarta xml everywhere and update PDFbox (#8521)
  Bump flexmark-ext-gfm-tasklist from 0.62.2 to 0.64.0 (#8515)
  Bump xmlunit-core from 2.8.4 to 2.9.0 (#8519)
  Bump org.openjfx.javafxplugin from 0.0.11 to 0.0.12 (#8518)
  Bump mockito-core from 4.3.0 to 4.3.1 (#8520)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@koppor koppor Awaiting requested review from koppor

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

URL field replaces '+' character with ' ' (white space) breaking URL
3 participants
@alexandre-cremieux @Siedlerchr @koppor