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

Add PropertyString, cleanHtml helper and escapeOrCleanHtml helper. #4004

Draft
wants to merge 14 commits into
base: dev
Choose a base branch
from
Draft
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"composer/package-versions-deprecated": "1.11.99.5",
"composer/semver": "3.4.3",
"endroid/qr-code": "5.1.0",
"ezyang/htmlpurifier": "4.17.0",
"guzzlehttp/guzzle": "7.9.2",
"jaybizzle/crawler-detect": "^1.2",
"laminas/laminas-cache": "3.12.2",
Expand Down
65 changes: 63 additions & 2 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions config/vufind/config.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2641,3 +2641,10 @@ description = "The REST API provides access to search functions and records cont
; By default, VuFind sorts text in a locale-agnostic way; if this setting is
; turned on, the current user-selected locale will impact sort order.
;use_locale_sorting = true

; By default HTML is not allowed in record fields, but support for sanitized HTML
; can be enabled per field type.
[HTML_Fields]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might be able to make this a little more clear. A few suggestions:

1.) I wonder if the top comment could be something more like "Data retrieved from records is escaped by default, on the assumption that it does not contain HTML. This section can be used to allow limited HTML in specific contexts; set values to true to allow HTML, and leave false to escape content."

2.) I found the "HTML_Fields" section name a bit confusing. Maybe something like Allowed_HTML_Contexts would be better (not that I love that either). I just wonder if "field" might be too limiting, so I rather prefer "context" -- and since all the settings are about turning something on, having a verb seems potentially helpful.

3.) Should we put comments above all the switches clarifying what the contexts really mean?

;title = false
;title-alt = false
;summary = false
4 changes: 4 additions & 0 deletions module/VuFind/src/VuFind/Content/Summaries/Demo.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

namespace VuFind\Content\Summaries;

use VuFind\String\PropertyString;

/**
* Demo (fake data) summaries content loader.
*
Expand Down Expand Up @@ -56,6 +58,8 @@ public function loadByIsbn($key, \VuFindCode\ISBN $isbnObj)
return [
'Demo summary key: ' . $key,
'Demo summary ISBN: ' . $isbnObj->get13(),
(new PropertyString('Demo non-HTML summary'))
->setHtml('<strong>Demo HTML Summary:</strong><ul><li>Item 1</li><li>Item 2</li></ul>'),
maccabeelevine marked this conversation as resolved.
Show resolved Hide resolved
];
}
}
4 changes: 3 additions & 1 deletion module/VuFind/src/VuFind/Content/Summaries/Syndetics.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

namespace VuFind\Content\Summaries;

use VuFind\String\PropertyString;

/**
* Syndetics Summaries content loader.
*
Expand Down Expand Up @@ -110,7 +112,7 @@ public function loadByIsbn($key, \VuFindCode\ISBN $isbnObj)
// If we have syndetics plus, we don't actually want the content
// we'll just stick in the relevant div
if ($this->usePlus) {
$summaries[] = $sourceInfo['div'];
$summaries[] = PropertyString::fromHtml($sourceInfo['div'])->setHtmlTrusted(true);
} else {
// Get the marc field for summaries. (520)
$nodes = $xmldoc2->GetElementsbyTagName('Fld520');
Expand Down
25 changes: 22 additions & 3 deletions module/VuFind/src/VuFind/RecordDriver/DefaultRecord.php
Original file line number Diff line number Diff line change
Expand Up @@ -1271,9 +1271,9 @@ public function getSystemDetails()
public function getSummary()
{
// We need to return an array, so if we have a description, turn it into an
// array (it should be a flat string according to the default schema, but we
// might as well support the array case just to be on the safe side:
return (array)($this->fields['description'] ?? []);
// array (it is a flat string in the default Solr schema, but we also
// support multivalued fields for other backends):
return $this->getFieldAsArray('description');
}

/**
Expand Down Expand Up @@ -1816,4 +1816,23 @@ public function getCoordinateLabels()
{
return (array)($this->fields['long_lat_label'] ?? []);
}

/**
* Get a field as an array
*
* @param string $field Field
*
* @return array
*/
protected function getFieldAsArray(string $field): array
{
// Make sure to return only non-empty values:
$value = $this->fields['description'] ?? '';
if ('' === $value) {
return [];
}
// Avoid casting since description can be a PropertyString too (and casting would return an array of object
// properties):
return is_array($value) ? $value : [$value];
demiankatz marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading
Loading