Skip to content

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

Merged
Siedlerchr merged 1 commit into
JabRef:mainfrom
alexandre-cremieux:fix-for-issue-8448
Feb 26, 2022
Merged

issue(8448): allow URL with + sign in entry tabs#8508
Siedlerchr merged 1 commit into
JabRef:mainfrom
alexandre-cremieux:fix-for-issue-8448

Conversation

@alexandre-cremieux

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

Copy link
Copy Markdown
Contributor

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

Siedlerchr commented Feb 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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

@Siedlerchr Siedlerchr left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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

alexandre-cremieux commented Feb 19, 2022

Copy link
Copy Markdown
Contributor Author

You are welcome. Happy to help.

koppor
koppor approved these changes Feb 20, 2022
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

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

Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
Comment thread src/main/java/org/jabref/logic/formatter/bibtexfields/CleanupUrlFormatter.java Outdated
koppor
koppor requested changes Feb 20, 2022
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

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

alexandre-cremieux commented Feb 21, 2022

Copy link
Copy Markdown
Contributor Author

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

@Siedlerchr

Siedlerchr commented Feb 21, 2022

Copy link
Copy Markdown
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