pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 443 forks source link

Support HTML markup in submission titles #2564

Closed stranack closed 1 year ago

stranack commented 7 years ago

Italics display in the TOC title (see The Poetic Life-Form: An Analysis on the Role of Elegy and Form in In Memoriam): http://journals.sfu.ca/courses/index.php/eng435/index

But not on the article page, where the tags themselves are displayed: http://journals.sfu.ca/courses/index.php/eng435/article/view/7

Edits

Metadata support details listed by @asmecher https://github.com/pkp/pkp-lib/issues/2564#issuecomment-904147340

PRs

App pkp-lib --> https://github.com/pkp/pkp-lib/pull/8584 ui-library --> https://github.com/pkp/ui-library/pull/252 ojs --> https://github.com/pkp/ojs/pull/3731 ops --> https://github.com/pkp/ops/pull/464 omp --> https://github.com/pkp/omp/pull/1329

Plugins OAI DC, MarcXML, RSS/ATOM [HTML Tag Support : NO] --> No Change Required DC(DublinCore) [HTML Tag Support : NO] --> No Change Required googleScholar[HTML Tag Support : NO] --> https://github.com/pkp/googleScholar/pull/11 [PR CLOSED] orcidProfile [HTML Tag Support : NO] --> https://github.com/pkp/orcidProfile/pull/231 crossref-ojs [HTML Tag Support : YES] --> https://github.com/pkp/crossref-ojs/pull/25 crossref-ops [HTML Tag Support : YES] --> https://github.com/pkp/crossref-ops/pull/22 medra[HTML Tag Support : NO] --> https://github.com/pkp/medra/pull/6 [PR CLOSED] DataCite [HTML Tag Support : NO] --> https://github.com/touhidurabir/ojs/commit/e0676e58571dd9bc2443c00b61c7e08aeceb7ef6 DOAJ [HTML Tag Support : NO] --> https://github.com/touhidurabir/ojs/commit/b080ef16f9b9da33fda40c5548b36e75704f7966 PubMed[HTML Tag Support : YES] --> https://github.com/touhidurabir/ojs/commit/ae06853af45791239cb7aa5582cdf7740acdb334 jatsTemplate [HTML Tag Support : YES] --> https://github.com/pkp/jatsTemplate/pull/22 oaiJats [HTML Tag Support : YES] --> https://github.com/pkp/oaiJats/pull/33 ONIX [HTML Tag Support : NO] --> https://github.com/pkp/omp/pull/1355

asmecher commented 7 years ago

In OJS 2.x, we used to allow HTML tags in article titles, but never (IIRC) dealt with what that meant downstream e.g. in OAI consumers. Plus it causes problems for users who legitimately want to write e.g. p < 0.05 in titles; they would need to write p &lt; 0.05 in the text field, which is totally unintuitive.

I suggest that if we do support this, we need to support it properly (e.g. with a rich text editor).

ajnyga commented 7 years ago

I have been looking for a rich text editor for a text input field, but to my surprise there seems to be none?

I suggest two solutions: 1) Allow a limited set of markdown in the title. The problem here is of course usability. (I think a Drupal module uses this approach for the same problem: https://www.drupal.org/project/html_title) 2) Let's use a textarea for the title fields instead of input and a custom set of tinymce buttons and a more strict html filter. (see http://wpsnipp.com/index.php/page/add-tinymce-editor-to-postpage-title-input-field/). This is a bit ugly, but will probably give better results.

asmecher commented 7 years ago

I'd prefer option 2 -- and we already have two presentations for TinyMCE ("extended" vs. normal or something like that), so adding a third configuration for minimal controls wouldn't be too invasive. I suspect titles would be a good driving use case for choosing which buttons should be available -- suggest B/I/U, perhaps super/subscript, and a button to edit the markup. Even just facilitating copy/paste into these fields in a rich format will be a big win, I think.

carzamora commented 6 years ago

Hi @asmecher, in biology this is a very important issue because the scientific name of any species have to be write in italics each time! And now in titles appear like name except in TOC, as say @stranack. For resolve this is not necessary implement TinyMCE, for now is sufficient with display the title like TOC, similar to OJS 2.4

What do you think?

asmecher commented 6 years ago

@t4x0n, the problem comes when we provide data to 3rd-party services/tools, like Google Scholar, OAI-PMH, CrossRef, etc. Most of those will expect plaintext, not HTML, and most authors will expect to have to enter plaintext, not HTML. If we standardize on accepting HTML, then it'll be necessary to strip tags from the feeds that go to 3rd-party standards. But without a tool like TinyMCE to facilitate the entry of HTML, we run the risk of having it strip legitimate uses of <, >, and & from titles.

carzamora commented 6 years ago

@asmecher I understand, but maybe an option can would whether for 3rd-party offer a XML file instead of HTML? ...I don't know, I am thinking in SciELO system, here an example article: http://www.scielo.cl/scielo.php?script=sci_arttext&pid=S0717-66432017000100001&lng=es&nrm=iso&tlng=en

asmecher commented 6 years ago

@t4x0n, I think the "proper" solution actually won't be so much work -- mostly it's just finding a configuration of TinyMCE that looks good when used to replace an <input type="text"> element.

carzamora commented 6 years ago

mmm ok! But @asmecher, that does not resolve the 3rd-party standards, right?

in OJS 2 we just put html tags for italics and was not a problem, but TinyMCE is probably a better solution, I am not familiar yet with the OJS code, so when I try for a similar solution to another issue I can’t was it :/ thanks for the reply!

asmecher commented 6 years ago

If we had TinyMCE support in the title fields...

Best of all worlds, as long as

carzamora commented 6 years ago

Excellent! I did not know that the system can convert html to plain text!

ajnyga commented 6 years ago

Hi,

Basically this should do it: https://github.com/pkp/pkp-lib/pull/3074 (of course needs some cleaning, the layout is problematic because of the prefix field and we should add rows="1" to make those fields smaller and maybe change the font size for those fields)

However, I noticed that there is a bug in the code that probably preventes the custom toolbar of being used.

For example the abstract field should use a special "rich" toolbar

It is called here: https://github.com/pkp/pkp-lib/blob/master/templates/submission/submissionMetadataFormTitleFields.tpl#L23

And defined here: https://github.com/pkp/pkp-lib/blob/master/js/classes/Handler.js#L700

The toolbar contents are defined here: https://github.com/pkp/pkp-lib/blob/master/js/controllers/SiteHandler.js#L150

However, at least for me, I am not seeing the bullist and numlist buttons in 3.1 although the abstract field should have them while they are included in the richToolbar?

@NateWr, I see that you have done work with the toolbar settings, can you confirm that this is a bug or am I missing something?

NateWr commented 6 years ago

It's been a long time since I fiddled with that so I don't have an idea off the top of my head. I'd check that the $FBV_rich variable is being set properly, and that the bullist and numlist buttons are the right names.

carzamora commented 6 years ago

@ajnyga for me is the same, I am not seeing the bullist and numlist buttons in 3.1, and upload image is not working for me either #3064

Some days ago, I was trying to generate another toolbar for this issue #2979, a toolbar without JBImages, but I can't be made it... maybe is the bug as you say

ajnyga commented 6 years ago

hmm, maybe it is missing plugins: https://community.tinymce.com/communityQuestion?id=90661000000If5QAAS

ajnyga commented 6 years ago

Ok, so the richToolbar is definitely loading, Just without those two buttons. I added advlist and lists to the list of plugins, but it does not seem to make a difference.

It is the richToolbar because it includes the superscript button which the default toolbar does not have.

But something is wrong there anyway, but I do not have the time to look at it just now.

So not sure why my pr above is not working. There is something fishy here...

mfelczak commented 6 years ago

Hi @asmecher, it also looks like we have inconsistent escaping of the article title, i.e. in the issue TOC we have $article->getLocalizedTitle()|strip_unsafe_html whereas in the article summary we have $article->getLocalizedTitle()|escape

asmecher commented 6 years ago

@ajnyga, the reason the lists tools weren't being displayed was that the lists plugin was missing from TinyMCE's initialization. I've added it: https://github.com/pkp/pkp-lib/commit/0c3d3ecbb31b336bc9e116b8e3a81d72a0224ce6

ajnyga commented 6 years ago

Hi, Yes, I tried that as well, but could not get it to work. Maybe it was a cache issue or something,

Anyway, I understood from the link above that advlist plugin is also needed. But is it workign without it as well?

asmecher commented 6 years ago

I got the tools to display with the changes I committed (on a control that was set to rich="extended"). You'll need to turn off minification (so your uncompiled Javascript is read instead of the compiled version) and flush the browser cache for changes to appear.

ajnyga commented 6 years ago

Hi @asmecher (and @NateWr, see especially the editor height issues below)

Here is a new pr. You were right that it was a cache issue that caused the earlier problems. https://github.com/pkp/pkp-lib/pull/3265

I did not test yet what ends up in the database when saving the form, or in what cases and how the html should be stripped from the titles. But these should not be that hard to add.

The visual layout is of course something that could be enhanced.

screenshot_3

Note that I set the min_height value for tinymce. I had difficulties in resizing the tinymyce height until I notice that there is a code that counts the height based on the number of textarea rows: https://github.com/pkp/pkp-lib/blob/master/js/controllers/SiteHandler.js#L220

However, if that value is below 100px there is a default min_height value in tinymce that returns the height back to 100px. But you can deal with that by setting a custom min_height in the settigns like I did now. I used 20px because now the setting of 1 row will return 20px height like you would expect based on the code here: https://github.com/pkp/pkp-lib/blob/master/js/controllers/SiteHandler.js#L220. BUT there could be cases where the editor will now return a smaller editor than was expected, for example the setting of 3 rows will return 60px when it used to return 100px which was the default min_height.

Another thing with the editor heights is that I noticed no visible affect when using for example height=$fbvStyles.height.TALL in the textarea settings. I did create a new ONELINE setting there, but maybe those could/should be removed altogether?

asmecher commented 6 years ago

Excellent start, @ajnyga! @NateWr, would you mind tinkering a little with this?

NateWr commented 6 years ago

Great work @ajnyga. I do have a few concerns about going ahead with this. Sorry to jump in so late.

  1. What happens when a user hits the enter key?
  2. What happens when a user pastes from a Word document or somewhere that carries with it HTML formatting (like styles)?
  3. Do we have a legitimate use-case for allowing bold and underlines?
  4. What exactly is being saved? In my experience, TinyMCE wraps text in <p> by default.

I understand that this is an important feature. But we're taking something bulletproof (a plain-text field) and introducing scope for lots of human error, some of it critical -- like if a bad title is handed off to citation/indexing handlers.

In my experience, taking something wide-ranging and trying to lock it down will lead us into a lot of maintenance. Every new device that comes out ends up breaking old hacks or supporting new types of entry we need to lock down.

I'd be tempted to explore with the Substance people what their experience is with needs like this, how they go about sanitizing a rich text area, and see whether there might be some option which is more limiting to users. Should I reach out to them?

ajnyga commented 6 years ago

Hi,

You are right that the input here would need a 100% sure way of cleaning the input.

  1. I think there should be the settings needed to take care of this in tinymce. Not sure what the right combination would be: https://www.tinymce.com/docs-3x/reference/configuration/Configuration3x@forced_root_block/ and https://www.tinymce.com/docs-3x/reference/configuration/Configuration3x@force_br_newlines/
  2. Probably this would help limit the copy pasted styles (but would need some testin) https://www.tinymce.com/docs-3x/reference/configuration/Configuration3x@valid_elements/
  3. No, I do not have, just for the three other settigns I added there. I realize that people will be tempted to bold the whole title now :D But maybe math would be something that people would like to see.
  4. I think setting this to false would take care of that: https://www.tinymce.com/docs-3x/reference/configuration/Configuration3x@forced_root_block/

As I mentioned above, I actually tried to look for a very simple text input editor for this purpose, but really could not find one. So tinymce (or something similar) seemed like the only alternative. By all means do ask the Substance people about this, because they have probably talked about similar use cases. For us (journal.fi) this is not a big issue. One of our journals does use italics in their title, but have been doing so since 2.x and just add the tags there.

NateWr commented 6 years ago

Are you able to try those config options? My sense from reading them is that the user will be still be able to insert line breaks (<br>), but worth trying it out.

Stripping attributes may be more or less successful. I remember it used to be awful at it but not sure if that's still the case.

ajnyga commented 6 years ago

Yes sure, but go ahead and contact Substance in case they have something better in mind.

NateWr commented 6 years ago

It says something that this old PKP issue is on the first page of results when I search for "tinymce restrict to single line": https://pkp.sfu.ca/bugzilla/bugslist/show_bug.cgi?id=6759

ajnyga commented 6 years ago

Hi @NateWr

https://github.com/pkp/pkp-lib/pull/3265

Referring to your list above

  1. Enter key is now disabled
  2. I added a list of valid tags. Even if you paste text that is divided to paragraphs or has linebreaks, the editor will remove those upon copying. Note that this would have to be tested with other browsers as well, not just Firefox. Also, I would be incluned to add a PHP filter that would be applied upon saving the data to make sure that only the limited tags will end up in the database.
  3. I removed bold and underline. If requrested, these can be added easily
  4. The default wrapping <p> is now removed form the oneline editor.

So I think that this is now working fairly well.

NateWr commented 6 years ago

:tada: Let's remove the "powered by" too. I think it's a config. We can keep them for the big ones, but the less clutter the better on these wee fields.

Also, let's explore inline mode to reduce the iframes we're loading as well as the space the fields take up in a form.

ajnyga commented 6 years ago

I added the branding setting, but it is a bit weird, because it affects all the editors, also the one in the abstract. I am not sure why it does so although I am only applying it to the oneline editor.

With the inline mode, I am not sure what you have in mind? Are you thinking that there would be a default text like "insert title here" inside a div and that the editor would be enabled upon clicking that div? Would that work with the current solution you have for multilingual fields?

NateWr commented 6 years ago

My assumption is that the inline mode will do two things:

  1. It will not use an iframe which might improve out performance.
  2. It will not display a toolbar by default. Only when the field is focused will the toolbar "hover" above the cursor. So when not in focus, the field should appear exactly like a regular text input.

If that doesn't work out, or you have trouble getting it going, let me know and I'll try to play around with it.

ajnyga commented 6 years ago

So I tried to add div to submissionMetadataFormTitleFields.tpl and added settings.inline = true; and settings.selector = "#divname" to Handler.js to the custom oneline editor settings I have added there, but this was not enough. I am not sure what was missing.

NateWr commented 6 years ago

To get the inline working, we need to use a <div> (or any block element) instead of a <textarea>. I was able to get this going with a few changes. It looks like your branch is a bit out-dated so I wasn't able to issue a clean PR, but you can just cherry-pick this commit in:

https://github.com/NateWr/pkp-lib/commit/5f1460c94c026466d82a76db1afb7caa6957b8fb

It still needs some work. Here's what it looks like:

selection_027

So still todo:

NateWr commented 6 years ago

Oh, I see now what you said about every tinymce going inline... Hmm...

NateWr commented 6 years ago

Ok, so we just needed to explicitly set inline to false each time we load the tinymce, otherwise the old value in the settings lingered. You can cherry-pick this commit in: https://github.com/NateWr/pkp-lib/commit/eeb646a0b1f22601aa778b507e05020c5777cd79

ajnyga commented 6 years ago

Hi @NateWr

See https://github.com/pkp/pkp-lib/pull/3544 for the latest pr. It seems to be working with one locale, but two form locales caused an error. Did not hae time to check it closer yet...

edit: was able to fix the multilingual support by changing the jquery selector. One small problem though: if you click on the title field the editor buttons appear, and when you leave the field, the editor buttons do not disappear. They do disappers if you edit the secondary locale last. So it probably needs a secondary function for hiding the editor buttons?

edit2: this commit https://github.com/pkp/pkp-lib/pull/3544/commits/a6e84d93c17b003dc6a4d8c764976d5629956e55#diff-83df7d7f7c760e14fa91323920849101 solves to problem partially. The toolbar is now hidden, but it is not initialized if you try to focus it right after that. If you click something else and try again it works. So it needs an additional show but could not figure out the right place and/or the right selector. You probably know the multilingual popover code a lot better...

nayttokuva 2018-3-31 kello 18 55 27
carzamora commented 6 years ago

maybe you know that, but... in OJS 3.1.1 the titles with italics look like this yet: image I think this was solved in this version

asmecher commented 6 years ago

Yes, italics/HTML in article titles isn't supported yet.

jmacgreg commented 5 years ago

Hi all, just checking on this. Has there been any progress on this, or on #3544?

ajnyga commented 5 years ago

i think i got that almost ready back then, only some validation things were not working. Can't promise that I have time to return to this right now, but should not take much work when I have the time. Anyone can step in and continue of course.

NateWr commented 5 years ago

Just a heads-up here: the title and subtitle fields will be migrated to the new Vue.js forms system as part of some UI/UX changes related to versioning. So this issue will likely be addressed in that context, as a new field type in the UI Library (which also uses TinyMCE, so we should be able to draw on the work done here).

ajnyga commented 5 years ago

thanks, been kind of waiting for that.

carzamora commented 4 years ago

Hi all, to understand what is the actual state of this issue: Will be possible to implement TinyMCE in titles in OJS 3.1.x or in OJS 3.2 because the migrate to the new Vue.js?

NateWr commented 4 years ago

Unfortunately, I don't think it will make it into OJS 3.2.

carzamora commented 4 years ago

ok, thanks @NateWr but please do not forget this issue for the future ;)

TimBoxH commented 4 years ago

Hi @NateWr, from my perspective as a scientist, the ability to italicize words or letters in article titles is of great importance. Is there a chance to prioritize this feature and implement it already in OJS 3.2? I think the community really needs it. We (currently hosting eight journals) got frequently asked for the feature. Cheers, Tim

NateWr commented 4 years ago

It's too late for 3.2 but I've applied the Community Priority label to increase the likelihood that this will get attention in the future.

ajnyga commented 4 years ago

I almost got this working with the old forms. Now that the new forms are implemented will try to solve this using them. We have a need for it as well.

fgnievinski commented 4 years ago

@t4x0n, the problem comes when we provide data to 3rd-party services/tools, like Google Scholar, OAI-PMH, CrossRef, etc. Most of those will expect plaintext, not HTML, and most authors will expect to have to enter plaintext, not HTML. If we standardize on accepting HTML, then it'll be necessary to strip tags from the feeds that go to 3rd-party standards. But without a tool like TinyMCE to facilitate the entry of HTML, we run the risk of having it strip legitimate uses of <, >, and & from titles.

CrossRef seems friendly to basic markup in the title -- bold, italic, underline, overline, superscript, subscript, small caps, and typewriter text (monospaced font):

https://www.crossref.org/education/content-registration/crossrefs-metadata-deposit-schema/face-markup/

fgnievinski commented 4 years ago

Just to note that superscript/subscript and some mathematics seem to be already supported via Unicode (typeset externally then pasted into OJS), see for example:

https://forum.pkp.sfu.ca/t/add-superscript-to-journal-title/30469/4

So it's mainly bold and italics that would seem to have no support so far.

denismaier commented 4 years ago

I really second this request. I'm currently preparing the first issue of a new journal, and they frequently refer to other titles in their titles, or they have transliterations in their titles.