8000 fix: Structure board update sometimes failed to add all interactive elements by fsbraun · Pull Request #8227 · django-cms/django-cms · GitHub
[go: up one dir, main page]
Skip to content

fix: Structure board update sometimes failed to add all interactive elements #8227

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 14, 2025
Merged

fix: Structure board update sometimes failed to add all interactive elements #8227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ea28e5
fix: Scan plugin data after structure mode Xhr load
fsbraun May 13, 2025
06002f1
Fix test
fsbraun May 13, 2025
0266ce2
fix js linting issues
fsbraun May 13, 2025
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
32 changes: 14 additions & 18 deletions cms/static/cms/js/modules/cms.structureboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,22 +395,20 @@ class StructureBoard {
CMS.settings.states = Helpers.getSettings().states;

const bodyRegex = /<body[\S\s]*?>([\S\s]*)<\/body>/gi;
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Remove the global flag from bodyRegex to avoid stateful exec issues

RegExp.exec with g is stateful and can return null on reuse; remove the g flag since only a single match is needed.

Suggested change
const bodyRegex = /<body[\S\s]*?>([\S\s]*)<\/body>/gi;
const bodyRegex = /<body[\S\s]*?>([\S\s]*)<\/body>/i;

const body = $(bodyRegex.exec(contentMarkup)[1]);
const body = document.createElement('div'); // Switch to plain JS due to problem with $(body)

const structure = body.find('.cms-structure-content');
const toolbar = body.find('.cms-toolbar');
const scripts = body.filter(function() {
return $(this).is('[type="text/cms-template"]'); // cms scripts
});
const pluginIds = this.getIds(body.find('.cms-draggable'));
const pluginDataSource = body.filter('script[data-cms]').toArray()
.map(script => script.textContent || '').join();
body.innerHTML = bodyRegex.exec(contentMarkup)[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): Guard against null return from exec before indexing

Assign bodyRegex.exec(contentMarkup) to a variable and verify it’s not null before accessing [1] to prevent a runtime error when there’s no match.


const structure = $(body.querySelector('.cms-structure-content'));
const toolbar = $(body.querySelector('.cms-toolbar'));
const scripts = $(body.querySelector('[type="text/cms-template"]')); // cms scripts
const pluginIds = this.getIds($(body.querySelectorAll('.cms-draggable')));
const pluginData = StructureBoard._getPluginDataFromMarkup(
pluginDataSource,
body,
pluginIds
);

Plugin._updateRegistry(pluginData.map(([, data]) => data));
Plugin._updateRegistry(pluginData);

CMS.API.Toolbar._refreshMarkup(toolbar);

Expand Down Expand Up @@ -1744,21 +1742,19 @@ class StructureBoard {
*
* @method _getPluginDataFromMarkup
* @private
* @param {String} markup
* @param {Node} body
* @param {Array<Number | String>} pluginIds
* @returns {Array<[String, Object]>}
*/
static _getPluginDataFromMarkup(markup, pluginIds) {
static _getPluginDataFromMarkup(body, pluginIds) {
return compact(
pluginIds.map(pluginId => {
// oh boy
const regex = new RegExp(`CMS._plugins.push\\((\\["cms\-plugin\-${pluginId}",[\\s\\S]*?\\])\\)`, 'g');
const matches = regex.exec(markup);
const pluginData = body.querySelector(`#cms-plugin-${pluginId}`);
let settings;

if (matches) {
if (pluginData) {
try {
settings = JSON.parse(matches[1]);
settings = JSON.parse(pluginData.textContent);
} catch (e) {
settings = false;
}
Expand Down
28 changes: 14 additions & 14 deletions cms/tests/frontend/unit/cms.structureboard.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2424,42 +2424,42 @@ describe('CMS.StructureBoard', function() {
});

describe('_getPluginDataFromMarkup()', () => {
const wrap = markup => $(`<div>${markup}</div>`)[0];
[
{
args: ['', [1, 2, 3]],
args: [wrap(''), [1, 2, 3]],
expected: []
},
{
args: ['whatever', []],
args: [wrap('whatever'), []],
expected: []
},
{
args: ['CMS._plugins.push(["cms-plugin-4",{"plugin_id":"4"}]);', [1, 2, 3]],
args: [wrap('<script id="cms-plugin-4">{"plugin_id":"4"}</script>'), [1, 2, 3]],
expected: []
},
{
args: ['CMS._plugins.push(["cms-plugin-4",{"plugin_id":"4"}]);', [1, 2, 4]],
expected: [['cms-plugin-4', { plugin_id: '4' }]]
args: [wrap('<script id="cms-plugin-4">{"plugin_id":"4"}</script>'), [1, 2, 4]],
expected: [{ plugin_id: '4' }]
},
{
args: [
`CMS._plugins.push(["cms-plugin-4",{"plugin_id":"4"}]);
CMS._plugins.push(["cms-plugin-10", { "plugin_id": "meh"}]);`, [1, 2, 10]],
expected: [['cms-plugin-10', { plugin_id: 'meh' }]]
wrap('<script id="cms-plugin-4">{"plugin_id":"4"}</script>' +
'<script id="cms-plugin-10">{"plugin_id":"meh"}</script>'),
[1, 2, 10]],
expected: [{ plugin_id: 'meh' }]
},
{
args: ['CMS._plugins.push(["cms-plugin-4",{plugin_id:"4"}])', [4]],
args: [wrap('<script id="cms-plugin-4">{plugin_id:"4"}</script>'), [4]],
expected: []
},
{
args: ['CMS._plugins.push(["cms-plugin-4",not a json :(]);', [4]],
args: [wrap('<script id="cms-plugin-4">not a json :(]</script>'), [4]],
expected: []
},
{
args: [`CMS._plugins.push(["cms-plugin-4", {
"something": 1
}])`, [4]],
expected: [['cms-plugin-4', { something: 1 }]]
args: [wrap('<script id="cms-plugin-4">{"something":1}</script>'), [4]],
expected: [{ something: 1 }]
}
].forEach((test, i) => {
it(`extracts plugin data from markup ${i}`, () => {
Expand Down
Loading
0