Skip to content

Fix "&" on previews#5786

Merged
tobiasdiez merged 4 commits into
JabRef:masterfrom
jjsfernandez:fix-ampersand-preview
Dec 31, 2019
Merged

Fix "&" on previews#5786
tobiasdiez merged 4 commits into
JabRef:masterfrom
jjsfernandez:fix-ampersand-preview

Conversation

@jjsfernandez

@jjsfernandez jjsfernandez commented Dec 26, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Proposed solution for #3840

I deleted a line where the & was getting escaped when generating the preview, apparently CSL already handles some kind of escaping.

The following entry

@Misc{b1111,
  author = {b, a},
  title  = {c \& d},
  year   = {1111},
}

generates this HTML

<div class="csl-entry">
  <div class="csl-left-margin">[1]</div><div class="csl-right-inline"> a b, ôc &#38; d.ö 1111.</div>
</div>

which renders in the PreviewLayout like this
ampersand

  • 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 bigger UI changes)
  • Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

@Siedlerchr

Siedlerchr commented Dec 26, 2019

Copy link
Copy Markdown
Member

I fear that will destroy the HTML export and other custom html exports.
The HTMLChars formatter is used there a lot.
https://docs.jabref.org/import-export/export/customexports#adding-a-custom-export-filter

Another idea which just came to my mind. Have you tried without the HTML chars formatter, e.g. if it works if you remove the HTML chars formatter in that step?

@jjsfernandez

jjsfernandez commented Dec 26, 2019

Copy link
Copy Markdown
Contributor Author

You are right! I removed the formatter from the CSL Adapter and it works just fine. I'll update the PR

@koppor

koppor commented Dec 26, 2019

Copy link
Copy Markdown
Member

@JosejeSinohui Thank you for working on JabRef - could you please merge upstream/master so that our automated checks are triggered on your PR?

@koppor

koppor commented Dec 26, 2019

Copy link
Copy Markdown
Member

Would it be possible to add test cases here? You already provided a snippet. - The reason is that in one year someone touching CSLAdapter won't know that the HtmlCharsFormatter was removed and will re-add it again. There won't be any automatism preventing that.

@jjsfernandez

jjsfernandez commented Dec 30, 2019

Copy link
Copy Markdown
Contributor Author

Hi, sorry for the delay. I will add tests and update my PR.

@tobiasdiez

tobiasdiez commented Dec 31, 2019

Copy link
Copy Markdown
Member

Perfect, thanks for the quick followup!

@tobiasdiez tobiasdiez merged commit 9474f5a into JabRef:master Dec 31, 2019
Siedlerchr added a commit that referenced this pull request Jan 4, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into customizeEntrydlg
70147eb 
* upstream/master:
  Add select all buttons to change dialog (#5803)
  Squashed 'src/main/resources/csl-locales/' changes from 9785a6e358..a3e8843f75
  Squashed 'src/main/resources/csl-styles/' changes from 49a1841..b2fbe15
  Fix CSL update
  Add new authors
  Remove ACM Fetcher in one more check
  Adapt WebFetchersTest
  Disable ACM Fetcher
  Try to fix branch name on cleanup pr action
  Remove obsolete class (#5801)
  Try to use right ref
  Run tests only once at pull requets
  Add ignored dependency
  Fix "&" on previews (#5786)
  [WIP] Batch Insert entries (#5691)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jjsfernandez @Siedlerchr @koppor @tobiasdiez