Skip to content

Move authoring UI inline JS handlers to data attributes #168

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

Open
snoopdave wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Move authoring UI inline JS handlers to data attributes #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f9bfdd
Move authoring UI inline JS handlers to data attributes
snoopdave Aug 29, 2026
713aba2
Double-quote authoring UI media attributes and tighten the sink audit
snoopdave Aug 29, 2026
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
13 changes: 9 additions & 4 deletions app/src/main/java/org/apache/roller/weblogger/ui/struts2/ajax/ThemeDataServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.roller.weblogger.ui.struts2.ajax;

import org.apache.commons.text.StringEscapeUtils;
import org.apache.roller.weblogger.WebloggerException;
import org.apache.roller.weblogger.business.WebloggerFactory;
import org.apache.roller.weblogger.business.themes.SharedTheme;
Expand Down Expand Up @@ -80,17 +81,21 @@ public void doGet(
}
for (Iterator<SharedTheme> it = themes.iterator(); it.hasNext();) {
SharedTheme theme = it.next();
// Theme metadata comes from theme.xml, which an operator can edit
// or install; escape it so a quote or newline cannot break out of
// the string and produce malformed JSON.
pw.print(" { \"id\" : \"");
pw.print(theme.getId());
pw.print(StringEscapeUtils.escapeJson(theme.getId()));
pw.print("\", ");
pw.print("\"name\" : \"");
pw.print(theme.getName());
pw.print(StringEscapeUtils.escapeJson(theme.getName()));
pw.print("\", ");
pw.print("\"description\" : \"");
pw.print(theme.getDescription());
pw.print(StringEscapeUtils.escapeJson(theme.getDescription()));

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CreateWeblog.jsp:118 consumes this same field with $('#themedescription').html(data.description), so a theme description is inert on Theme Edit but still rendered as markup on Create Weblog. Worth switching that call to .text() in this PR, since the audit test only scans jsps/editor and won't notice it.

pw.print("\", ");
pw.print("\"previewPath\" : \"");
pw.print("/themes" + "/" + theme.getId() + "/" + theme.getPreviewImage().getPath());
pw.print(StringEscapeUtils.escapeJson(
"/themes" + "/" + theme.getId() + "/" + theme.getPreviewImage().getPath()));
pw.print("\" }");
if (it.hasNext()) {
pw.println(", ");
Expand Down
35 changes: 23 additions & 12 deletions app/src/main/webapp/WEB-INF/jsps/editor/Bookmarks.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and

<td align="center">

<a href="#" onclick="editBookmark(
'<s:property value="#bookmark.id"/>',
'<s:property value="#bookmark.name"/>',
'<s:property value="#bookmark.url"/>',
'<s:property value="#bookmark.feedUrl"/>',
'<s:property value="#bookmark.description"/>',
'<s:property value="#bookmark.image"/>' )">
<a href="#" class="bookmark-edit-link"
data-bookmark-id="<s:property value="#bookmark.id"/>"
data-bookmark-name="<s:property value="#bookmark.name"/>"
data-bookmark-url="<s:property value="#bookmark.url"/>"
data-bookmark-feed-url="<s:property value="#bookmark.feedUrl"/>"
data-bookmark-description="<s:property value="#bookmark.description"/>"
data-bookmark-image="<s:property value="#bookmark.image"/>">
<span class="glyphicon glyphicon-edit"></span>
</a>

Expand Down Expand Up @@ -338,7 +338,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and

function confirmDeleteFolder() {
$('#boomarks_delete_folder_folderId').val($('#bookmarks_folderId:first').val());
$('#deleteBlogrollName').html('<s:property value="%{folder.name}"/>');
$('#deleteBlogrollName').text('<s:property value="%{folder.name}"/>');

@mraible mraible Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

<s:property> HTML-escapes with escapeHtml4, which doesn't touch ', so a folder named Matt's Links renders as .text('Matt's Links') and the SyntaxError takes out every function in this <script> block (delete/rename buttons stop working). And now that it's .text(), Tom & Jerry displays as Tom &amp; Jerry. Use <s:property value="%{folder.name}" escapeJavaScript="true" escapeHtml="false"/> here, or read the name from a data- attribute on the trigger element instead of inlining it in JS.

$('#delete-blogroll-modal').modal({show: true});
}

Expand Down Expand Up @@ -700,12 +700,23 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
$('#bookmarkEdit_bean_image:first').val('');
$('#bookmarkEdit_bean_feedUrl:first').val('');

$('#subtitle_folder_name:first').html(originalName);
$('#subtitle_folder_name:first').text(originalName);

$('#addedit-bookmark-modal').modal({show: true});
}


// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('click', '.bookmark-edit-link', function (event) {
event.preventDefault();
editBookmark($(this).attr('data-bookmark-id'),
$(this).attr('data-bookmark-name'),
$(this).attr('data-bookmark-url'),
$(this).attr('data-bookmark-feed-url'),
$(this).attr('data-bookmark-description'),
$(this).attr('data-bookmark-image'));
});

function editBookmark(id, name, url, feedUrl, description, image) {

var saveBookmarkButton = $('#save_bookmark:first');
Expand All @@ -725,7 +736,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
$('#bookmarkEdit_bean_description:first').val(description);
$('#bookmarkEdit_bean_image:first').val(image);

$('#subtitle_folder_name:first').html(originalName);
$('#subtitle_folder_name:first').text(originalName);

$('#addedit-bookmark-modal').modal({show: true});
}
Expand Down Expand Up @@ -770,7 +781,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
elem.removeClass("alert-info");
elem.removeClass("alert-danger");
elem.addClass("alert-success");
elem.html(message);
elem.text(message);

} else {
saveBookmarkButton.attr("disabled", true);
Expand All @@ -789,7 +800,7 @@ We used to call them Bookmarks and Folders, now we call them Blogroll links and
elem.removeClass("alert-info");
elem.removeClass("alert-success");
elem.addClass("alert-danger");
elem.html(message);
elem.text(message);
}
}

Expand Down
63 changes: 45 additions & 18 deletions app/src/main/webapp/WEB-INF/jsps/editor/Categories.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@
<s:set var="categoryName" value="#category.name" />
<s:set var="categoryDesc" value="#category.description" />
<s:set var="categoryImage" value="#category.image" />
<a href="#" onclick="showCategoryEditModal(
'<s:property value="categoryId" />',
'<s:property value="categoryName"/>',
'<s:property value="categoryDesc"/>',
'<s:property value="categoryImage"/>' )">
<a href="#" class="category-edit-link"
data-category-id="<s:property value="categoryId" />"
data-category-name="<s:property value="categoryName"/>"
data-category-desc="<s:property value="categoryDesc"/>"
data-category-image="<s:property value="categoryImage"/>">
<span class="glyphicon glyphicon-edit"></span>
</a>

Expand All @@ -71,10 +71,10 @@
<s:set var="categoryId" value="#category.id" />
<s:set var="categoryName" value="#category.name" />
<s:set var="categoryInUse" value="#category.inUse.toString()" />
<a href="#" onclick="showCategoryDeleteModal(
'<s:property value="categoryId" />',
'<s:property value="categoryName" />',
<s:property value="categoryInUse"/> )" >
<a href="#" class="category-delete-link"
data-category-id="<s:property value="categoryId" />"
data-category-name="<s:property value="categoryName" />"
data-category-in-use="<s:property value="categoryInUse"/>">
<span class="glyphicon glyphicon-trash"></span>
</a>

Expand Down Expand Up @@ -288,7 +288,7 @@
function showCategoryDeleteModal( id, name, inUse ) {
$('#categoryRemove_removeId').val(id);
$('#categoryEdit_bean_name').val(name);
$('#category-name').html(name);
$('#category-name').text(name);
if ( inUse ) {
$('#category-in-use').css('display','block');

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pre-existing, but since this function is being reworked: lines 294/297 toggle #category-emtpy while the element is #category-empty (line 266), so the "no entries in this category" message never shows.

$('#category-emtpy').css('display', 'none');
Expand All @@ -301,14 +301,15 @@
}

function populateCategorySelect(removeId) {
const allCategories = [];

<s:iterator value="allCategories" var="category">
allCategories.push({
id: '<s:property value="#category.id"/>',
name: '<s:property value="#category.name"/>'
});
</s:iterator>
// Category names are author-supplied. They are rendered into data
// attributes below and read back as text here, so no name is ever
// parsed as JavaScript.
const allCategories = $('#category-option-data .category-option').map(function () {
return {
id: $(this).attr('data-category-id'),
name: $(this).attr('data-category-name')
};
}).get();

const select = $('#categoryRemove_targetCategoryId');
select.empty();
Expand All @@ -319,4 +320,30 @@
});
}

// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('click', '.category-edit-link', function (event) {
event.preventDefault();
showCategoryEditModal($(this).attr('data-category-id'),
$(this).attr('data-category-name'),
$(this).attr('data-category-desc'),
$(this).attr('data-category-image'));
});

$(document).on('click', '.category-delete-link', function (event) {
event.preventDefault();
showCategoryDeleteModal($(this).attr('data-category-id'),
$(this).attr('data-category-name'),
$(this).attr('data-category-in-use') === 'true');
});

</script>

<%-- Source data for the "move entries to" select, carried as escaped
attributes rather than generated JavaScript literals. --%>
<div id="category-option-data" style="display:none">

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the .category-edit-link anchors above already carry data-category-id / data-category-name for every category, so populateCategorySelect could read those instead of maintaining a second serialised copy.

<s:iterator value="allCategories" var="category">
<span class="category-option"
data-category-id="<s:property value="#category.id"/>"
data-category-name="<s:property value="#category.name"/>"></span>
</s:iterator>
</div>
15 changes: 11 additions & 4 deletions app/src/main/webapp/WEB-INF/jsps/editor/Entries.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,9 @@
<td>
<s:set var="postId" value="#post.id" />
<s:set var="postTitle" value="#post.title" />
<a href="#"
onclick="showDeleteModal('<s:property value="postId" />', '<s:property value="postTitle"/>' )">
<a href="#" class="entry-delete-link"
data-post-id="<s:property value="postId" />"
data-post-title="<s:property value="postTitle"/>">
<span class="glyphicon glyphicon-trash"
data-toggle="tooltip" data-placement="top" title="<s:text name='generic.delete'/>">
</span>
Expand Down Expand Up @@ -238,9 +239,15 @@

<script>
function showDeleteModal( postId, postTitle ) {
$('#postIdLabel').html(postId);
$('#postTitleLabel').html(postTitle);
$('#postIdLabel').text(postId);
$('#postTitleLabel').text(postTitle);
$('#removeId').val(postId);
$('#delete-entry-modal').modal({show: true});
}

// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('click', '.entry-delete-link', function (event) {
event.preventDefault();
showDeleteModal($(this).attr('data-post-id'), $(this).attr('data-post-title'));
});
</script>
15 changes: 11 additions & 4 deletions app/src/main/webapp/WEB-INF/jsps/editor/EntryEdit.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,10 @@

<%-- delete --%>
<span style="float:right">
<input class="btn btn-danger" type="button"
<input class="btn btn-danger entry-delete-button" type="button"
value="<s:text name='weblogEdit.deleteEntry'/>"
onclick="showDeleteModal('<s:property value="entry.id" />', '<s:property value="entry.title"/>' )">
data-post-id="<s:property value="entry.id" />"
data-post-title="<s:property value="entry.title"/>">
</span>
</s:if>

Expand Down Expand Up @@ -436,10 +437,16 @@
});

function showDeleteModal(postId, postTitle) {
$('#postIdLabel').html(postId);
$('#postTitleLabel').html(postTitle);
$('#postIdLabel').text(postId);
$('#postTitleLabel').text(postTitle);
$('#removeId').val(postId);
$('#delete-entry-modal').modal({show: true});
}

// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('click', '.entry-delete-button', function (event) {
event.preventDefault();
showDeleteModal($(this).attr('data-post-id'), $(this).attr('data-post-title'));
});

</script>
9 changes: 7 additions & 2 deletions app/src/main/webapp/WEB-INF/jsps/editor/MediaFileAddSuccess.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@
<div class="row">

<div class="col-md-1">
<input type="radio" name="enclosure"
onchange="setEnclosure('<s:property value="%{#newFile.permalink}"/>')"/>
<input type="radio" name="enclosure" class="enclosure-choice"
data-enclosure-url="<s:property value="%{#newFile.permalink}"/>"/>
</div>

<div class="col-md-11">
Expand Down Expand Up @@ -201,6 +201,11 @@
return false;
}

// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('change', '.enclosure-choice', function () {
setEnclosure($(this).attr('data-enclosure-url'));
});

function setEnclosure(url) {
$("#enclosureURL").get(0).value = url;
if (isImageChecked()) {
Expand Down
23 changes: 15 additions & 8 deletions app/src/main/webapp/WEB-INF/jsps/editor/MediaFileImageChooser.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,16 @@
<li class="align-images"
onmouseover="highlight(this, true)" onmouseout="highlight(this, false)">

<div class="mediaObject"
onclick="onSelectMediaFile('<s:property value="#mediaFile.name"/>',
'<s:property value="%{mediaFileURL}"/>',
'<s:property value="#mediaFile.isImageFile()"/>')">
<div class="mediaObject mediafile-select-target"
data-mediafile-name="<s:property value="#mediaFile.name"/>"
data-mediafile-url="<s:property value="%{mediaFileURL}"/>"
data-mediafile-is-image="<s:property value="#mediaFile.isImageFile()"/>">

<s:if test="#mediaFile.imageFile">
<img border="0" src='<s:property value="%{mediaFileThumbnailURL}" />'
width='<s:property value="#mediaFile.thumbnailWidth"/>'
height='<s:property value="#mediaFile.thumbnailHeight"/>'
alt='<s:property value="#mediaFile.name" />'/>
<img border="0" src="<s:property value='%{mediaFileThumbnailURL}' />"
width="<s:property value='#mediaFile.thumbnailWidth'/>"
height="<s:property value='#mediaFile.thumbnailHeight'/>"
alt="<s:property value='#mediaFile.name' />"/>
</s:if>

<s:else>
Expand Down Expand Up @@ -118,6 +118,13 @@
window.parent.onSelectMediaFile(name, url, isImage);
}

// Values come from data-* attributes and are bound via delegated listeners.
$(document).on('click', '.mediafile-select-target', function () {
onSelectMediaFile($(this).attr('data-mediafile-name'),
$(this).attr('data-mediafile-url'),
$(this).attr('data-mediafile-is-image'));
});

function highlight(el, flag) {
if (flag) {
$(el).addClass("highlight");
Expand Down
Loading
Loading