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

Support HTML markup in submission titles #2564

Closed
stranack opened this issue Jun 2, 2017 · 84 comments
Closed

Support HTML markup in submission titles #2564

stranack opened this issue Jun 2, 2017 · 84 comments
Assignees
Labels
Community:2:Priority Any issue that has been identified through research or feedback as a major community priority. Enhancement:3:Major A new feature or improvement that will take a month or more to complete. Hosting Bug reports and feature requests from Publishing Services's hosted clients.
Milestone

Comments

@stranack
Copy link

stranack commented Jun 2, 2017

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 #2564 (comment)

PRs

App
pkp-lib --> #8584
ui-library --> pkp/ui-library#252
ojs --> pkp/ojs#3731
ops --> pkp/ops#464
omp --> pkp/omp#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] --> pkp/googleScholar#11 [PR CLOSED]
orcidProfile [HTML Tag Support : NO] --> pkp/orcidProfile#231
crossref-ojs [HTML Tag Support : YES] --> pkp/crossref-ojs#25
crossref-ops [HTML Tag Support : YES] --> pkp/crossref-ops#22
medra[HTML Tag Support : NO] --> pkp/medra#6 [PR CLOSED]
DataCite [HTML Tag Support : NO] --> touhidurabir/ojs@e0676e5
DOAJ [HTML Tag Support : NO] --> touhidurabir/ojs@b080ef1
PubMed[HTML Tag Support : YES] --> touhidurabir/ojs@ae06853
jatsTemplate [HTML Tag Support : YES] --> pkp/jatsTemplate#22
oaiJats [HTML Tag Support : YES] --> pkp/oaiJats#33
ONIX [HTML Tag Support : NO] --> pkp/omp#1355

@asmecher
Copy link
Member

asmecher commented Jun 2, 2017

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

ajnyga commented Jun 20, 2017

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

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

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

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

@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
Copy link
Member

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

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

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

  • Authors would be able to naively type <, >, and & characters and they'd be HTML-encoded properly
  • Authors could use the WYSIWYG tools to enter rich content without needing to know HTML, or paste them in from rich-capable sources
  • OJS would predictably know all stored titles were in HTML, so
    • it could down-convert to plaintext when needed (there are definitely a few hairy considerations here)
    • it could present them internally with rich content

Best of all worlds, as long as

  • TinyMCE can be persuaded to look/behave well as a single-line input, and
  • we can reliably convert HTML to text for formats not supporting HTML. (This is already implemented and used in various fields, but titles are so important that we may need to review how well this works.)

@carzamora
Copy link
Contributor

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

@ajnyga
Copy link
Collaborator

ajnyga commented Nov 17, 2017

Hi,

Basically this should do it: #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
Copy link
Contributor

NateWr commented Nov 17, 2017

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

@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
Copy link
Collaborator

ajnyga commented Nov 17, 2017

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

@ajnyga
Copy link
Collaborator

ajnyga commented Nov 17, 2017

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

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

@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: 0c3d3ec

@ajnyga
Copy link
Collaborator

ajnyga commented Nov 17, 2017

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

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

ajnyga commented Jan 12, 2018

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.
#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
Copy link
Member

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

@NateWr
Copy link
Contributor

NateWr commented Jan 15, 2018

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

ajnyga commented Jan 15, 2018

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

NateWr commented Jan 15, 2018

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

ajnyga commented Jan 15, 2018

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

@NateWr
Copy link
Contributor

NateWr commented Jan 15, 2018

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

ajnyga commented Jan 16, 2018

Hi @NateWr

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

NateWr commented Jan 16, 2018

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

asmecher pushed a commit that referenced this issue Feb 16, 2023
asmecher pushed a commit that referenced this issue Feb 16, 2023
asmecher pushed a commit that referenced this issue Feb 16, 2023
asmecher pushed a commit that referenced this issue Feb 16, 2023
asmecher pushed a commit that referenced this issue Feb 16, 2023
asmecher added a commit to pkp/crossref-ojs that referenced this issue Feb 16, 2023
pkp/pkp-lib#2564 Submission title and sub title with HTML tag handling
asmecher added a commit to pkp/crossref-ops that referenced this issue Feb 16, 2023
pkp/pkp-lib#2564 Submission title and sub title with HTML tag handling
asmecher pushed a commit to pkp/jatsTemplate that referenced this issue Feb 16, 2023
@asmecher
Copy link
Member

All merged except pkp/oaiJats#33 -- there's a comment there. Thanks, @touhidurabir!

touhidurabir added a commit to touhidurabir/pkp-oaiJats that referenced this issue Feb 16, 2023
@touhidurabir
Copy link
Member

@asmecher check updated the PR at pkp/oaiJats#33 for OAI Jats .

asmecher pushed a commit to pkp/oaiJats that referenced this issue Feb 16, 2023
)

* pkp/pkp-lib#2564 Added support for HTML tags in title and sub title

* pkp/pkp-lib#2564 Fixed HTML tags presentation in XML
@asmecher
Copy link
Member

All merged! Thanks again, @touhidurabir.

@touhidurabir
Copy link
Member

@asmecher please review the ONIX update for OMP at pkp/omp#1355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community:2:Priority Any issue that has been identified through research or feedback as a major community priority. Enhancement:3:Major A new feature or improvement that will take a month or more to complete. Hosting Bug reports and feature requests from Publishing Services's hosted clients.
Projects
Status: No status
Development

No branches or pull requests