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.

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

Fix "&" on previews #5786

Merged
merged 4 commits into from
Dec 31, 2019
Merged

Fix "&" on previews #5786

merged 4 commits into from
Dec 31, 2019

Conversation

jjsfernandez
Copy link
Contributor

@jjsfernandez jjsfernandez commented Dec 26, 2019

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

@Siedlerchr
Copy link
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
Copy link
Contributor Author

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

@koppor
Copy link
Member

koppor commented Dec 26, 2019

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

@koppor
Copy link
Member

koppor commented Dec 26, 2019

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
Copy link
Contributor Author

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

@tobiasdiez
Copy link
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
* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants