SAK-50813 Site titles with ampersands distorted during publishing

[hornersa]

Jira: https://sakaiproject.atlassian.net/browse/SAK-50813

This proposed fix is preliminary, showing one proposed solution (uncommented) and an alternative (commented out), acknowledging that there may be other approaches.

The first proposed solution (uncommented) is very specific, narrowly focused on fixing the issue with ampersands, which is a common character to be found in site titles (at least for our institution). This approach might valid/good-enough as I cannot think of other HTML meta-characters that are allowable for the Site Title (in Site Info > Edit Site Information); for example, greater than or less than characters are filtered out (when directly entered as is) by the time a revision to the title is to be confirmed (again, via Site Info > Edit Site Information). That said, the approach of this first proposed solution might have overlooked valid characters (which in turn might be resolved with additional StringUtils.replace statements, if there aren't many).

The other approach using convertFormattedTextToPlaintext (commented out) will resolve also the ampersand issue. It may also care for other HTML metacharacters overlooked. This approach however might be deemed too broad.

Once a consensus is reached among reviewers, I can revise this PR to implement the agreed upon approach prior to merging to trunk.

[ottenhoff]

The more-global, more-correct solution is always going to be preferred. It's clear that there should be no HTML entities in the site title whether it's an amp or an nbsp

[hornersa]

@ottenhoff - I concede that point. I also wasn't sure if convertFormattedTextToPlaintext had any other side effects; in other words, the title string is first run through processFormattedText and then the more global convertFormattedTextToPlaintext is run. Would this latter method 'undo' some crucial magic that processFormattedText implements?

[ottenhoff]

This just seems like a mistake in the bjones commit ... a site title should never have HTML, right? So then the choice of processFormattedText was wrong.

Feel free to add a personal test case to #13169

[hornersa]

Instead of processFormattedText (and the rest), would you suggest the following instead?

title = formattedText.stripHtmlFromText(title, false, true);

The last arg, stripEscapeSequences, needs to be true to prevent the display of html entities.

[ottenhoff]

yes, i would recommend doing exactly what is being done in SiteAction.java. it's great if you can centralize things into one method and when that is impossible or very difficult, then just copy the code:

			// site title is editable; cannot but null/empty after HTML stripping, and cannot exceed max length
			String titleOrig = params.getString("title");
			String titleStripped = formattedText.stripHtmlFromText(titleOrig, true, true);
			if (isSiteTitleValid(titleOrig, titleStripped, state)) {
				siteInfo.title = titleStripped;
			}

[ottenhoff]

@hornersa please close this if you aren't planning on finishing.