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

PRO-6587: access to fully rendered rich text widgets without giving up all access to other area properties or logging out #4806

Merged
merged 1 commit into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@
* Bumped `express-bearer-token` dependency to address a low-severity `npm audit` warning regarding noncompliant cookie names and values. Apostrophe
did not actually use any noncompliant cookie names or values, so there was no vulnerability in Apostrophe.
* Rich text "Styles" toolbar now has visually focused state.
* The `renderPermalinks` and `renderImages` methods of the `@apostrophecms/rich-text` module now correctly resolve the final URLs of page links and inline images in rich text widgets, even when the user has editing privileges. Formerly this was mistakenly prevented by logic intended to preserve the editing experience. The editing experience never actually relied on the
rendered output.

### Adds

* It's possible now to target the HMR build when registering via `template.append` and `template.prepend`. Use `when: 'hmr:public'` or `when: 'hmr:apos'` that will be evaluated against the current asset `options.hmr` configuration.
* Adds asset module option `options.modulePreloadPolyfill` (default `true`) to allow disabling the polyfill preload for e.g. external front-ends.
* Adds `bundleMarkup` to the data sent to the external front-end, containing all markup for injecting Apostrophe UI in the front-end.
* Warns users when two page types have the same field name, but a different field type. This may cause errors or other problems when an editor switches page types.
* The piece and page `GET` REST APIs now support `?render-areas=inline`. When this parameter is used, an HTML rendering of each widget is added to that specific widget in each area's `items` array as a new `_rendered` property. The existing `?render-areas=1` parameter is still supported to render the entire area as a single `_rendered` property. Note that this older option also causes `items` to be omitted from the response.

### Changes

Expand Down
30 changes: 26 additions & 4 deletions modules/@apostrophecms/area/index.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if you want to honor/fix the GitHub Action flag here.

Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@
setWidgetManager(name, manager) {
self.widgetManagers[name] = manager;
},
// Given the options passed to the area field, return the options passed
// to each widget type, indexed by widget name. This provides a consistent
// interface regardless of whether `options.widgets` or `options.groups`
// was used.
getWidgets(options) {
let widgets = options.widgets || {};

Expand Down Expand Up @@ -167,7 +171,11 @@
},
// Render the given `area` object via `area.html`, with the given `context`
// which may be omitted. Called for you by the `{% area %} custom tag.
async renderArea(req, area, _with) {
//
// If `inline` is true then the rendering of each widget is attached
// to the widget as a `_rendered` property, bypassing normal full-area
// HTML responses, and the return value of this method is `null`.
async renderArea(req, area, _with, { inline = false } = {}) {
if (!area._id) {
throw new Error('All areas must have an _id property in A3.x. Area details:\n\n' + JSON.stringify(area));
}
Expand All @@ -193,7 +201,7 @@
const manager = self.widgetManagers[name];
if (manager) {
choices.push({
name: name,

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 6)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 8)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 7)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 6)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 6)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 7)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 8)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 7)

Expected property shorthand

Check warning on line 204 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 8)

Expected property shorthand
icon: manager.options.icon,
label: options.addLabel || manager.label || `No label for ${name}`
});
Expand All @@ -212,6 +220,12 @@
// just use the helpers
self.apos.attachment.all(area, { annotate: true });
}
if (inline) {
for (const item of area.items) {
item._rendered = await self.renderWidget(req, item.type, item, widgets[item.type]);
}
return null;
}
return self.render(req, 'area', {
// TODO filter area to exclude big relationship objects, but
// not so sloppy this time please
Expand All @@ -226,7 +240,13 @@
// Replace documents' area objects with rendered HTML for each area.
// This is used by GET requests including the `render-areas` query
// parameter. `within` is an array of Apostrophe documents.
async renderDocsAreas(req, within) {
//
// If `inline` is true a rendering of each individual widget is
// added as an extra `_rendered` property of that widget, alongside
// its normal properties. Otherwise a rendering of the entire area
// is supplied as the `_rendered` property of that area and the
// `items` array is suppressed from the response.
async renderDocsAreas(req, within, { inline = false } = {}) {
within = Array.isArray(within) ? within : [];
let index = 0;
// Loop over the docs in the array passed in.
Expand Down Expand Up @@ -270,8 +290,10 @@
async function render(area, path, context, opts) {
const preppedArea = self.prepForRender(area, context, path);

const areaRendered = await self.apos.area.renderArea(req, preppedArea, context);

const areaRendered = await self.apos.area.renderArea(req, preppedArea, context, { inline });
if (inline) {
return;
}
_.set(context, [ path, '_rendered' ], areaRendered);
_.set(context, [ path, '_fieldId' ], undefined);
_.set(context, [ path, 'items' ], undefined);
Expand Down Expand Up @@ -412,7 +434,7 @@
}
_.set(doc, dotPath, {
metaType: 'area',
items: items

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 6)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 8)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 7)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 6)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 6)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 7)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 8)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 7)

Expected property shorthand

Check warning on line 437 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 8)

Expected property shorthand
});
return self.apos.doc.update(req, doc);
}
Expand Down Expand Up @@ -724,7 +746,7 @@
} else {
area = doc[name];
}
return self.isEmpty({ area: area });

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 6)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 8)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 7)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (20, 6)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 6)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 7)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (18, 8)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 7)

Expected property shorthand

Check warning on line 749 in modules/@apostrophecms/area/index.js

View workflow job for this annotation

GitHub Actions / build (22, 8)

Expected property shorthand
}
};
},
Expand Down
8 changes: 6 additions & 2 deletions modules/@apostrophecms/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,12 @@ module.exports = {
if (!result) {
throw self.apos.error('notfound');
}
if (self.apos.launder.boolean(req.query['render-areas']) === true) {
await self.apos.area.renderDocsAreas(req, [ result ]);
const renderAreas = req.query['render-areas'];
const inline = renderAreas === 'inline';
if (inline || self.apos.launder.boolean(renderAreas)) {
await self.apos.area.renderDocsAreas(req, [ result ], {
inline
});
}
// Attach `_url` and `_urls` properties
self.apos.attachment.all(result, { annotate: true });
Expand Down
16 changes: 12 additions & 4 deletions modules/@apostrophecms/piece-type/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,12 @@ module.exports = {
result.currentPage = query.get('page') || 1;
result.results = (await query.toArray())
.map(doc => self.removeForbiddenFields(req, doc));
if (self.apos.launder.boolean(req.query['render-areas']) === true) {
await self.apos.area.renderDocsAreas(req, result.results);
const renderAreas = req.query['render-areas'];
const inline = renderAreas === 'inline';
if (inline || self.apos.launder.boolean(renderAreas)) {
await self.apos.area.renderDocsAreas(req, result.results, {
inline
});
}
if (query.get('choicesResults')) {
result.choices = query.get('choicesResults');
Expand Down Expand Up @@ -291,8 +295,12 @@ module.exports = {
if (!doc) {
throw self.apos.error('notfound');
}
if (self.apos.launder.boolean(req.query['render-areas']) === true) {
await self.apos.area.renderDocsAreas(req, [ doc ]);
const renderAreas = req.query['render-areas'];
const inline = renderAreas === 'inline';
if (inline || self.apos.launder.boolean(renderAreas)) {
await self.apos.area.renderDocsAreas(req, [ doc ], {
inline
});
}
self.apos.attachment.all(doc, { annotate: true });
return doc;
Expand Down
32 changes: 13 additions & 19 deletions modules/@apostrophecms/rich-text-widget/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,8 @@ module.exports = {
},

// Quickly replaces rich text permalink placeholder URLs with
// actual, SEO-friendly URLs based on `widget._relatedDocs`
// actual, SEO-friendly URLs based on `widget._relatedDocs`.

linkPermalinks(widget, content) {
// "Why no regexps?" We need to do this as quickly as we can.
// indexOf and lastIndexOf are much faster.
Expand All @@ -756,13 +757,11 @@ module.exports = {
const left = content.lastIndexOf('<', i);
const href = content.indexOf(' href="', left);
const close = content.indexOf('"', href + 7);
if (!widget._edit) {
if ((left !== -1) && (href !== -1) && (close !== -1)) {
content = content.substring(0, href + 6) + doc._url + content.substring(close + 1);
} else {
// So we don't get stuck in an infinite loop
break;
}
if ((left !== -1) && (href !== -1) && (close !== -1)) {
content = content.substring(0, href + 6) + doc._url + content.substring(close + 1);
} else {
// So we don't get stuck in an infinite loop
break;
}
if (!updateTitle) {
continue;
Expand All @@ -777,11 +776,8 @@ module.exports = {
return content;
},
// Quickly replaces inline image placeholder URLs with
// actual, SEO-friendly URLs based on `widget._relatedDocs`
// actual, SEO-friendly URLs based on `widget._relatedDocs`.
linkImages(widget, content) {
if (widget._edit) {
return content;
}
// "Why no regexps?" We need to do this as quickly as we can.
// indexOf and lastIndexOf are much faster.
let i;
Expand All @@ -799,13 +795,11 @@ module.exports = {
const left = content.lastIndexOf('<', i);
const src = content.indexOf(' src="', left);
const close = content.indexOf('"', src + 6);
if (!widget._edit) {
if ((left !== -1) && (src !== -1) && (close !== -1)) {
content = content.substring(0, src + 5) + doc.attachment._urls[self.apos.modules['@apostrophecms/image'].getLargestSize()] + content.substring(close + 1);
} else {
// So we don't get stuck in an infinite loop
break;
}
if ((left !== -1) && (src !== -1) && (close !== -1)) {
content = content.substring(0, src + 5) + doc.attachment._urls[self.apos.modules['@apostrophecms/image'].getLargestSize()] + content.substring(close + 1);
} else {
// So we don't get stuck in an infinite loop
break;
}
}
}
Expand Down
Loading