Skip to content

XWIKI-24144: Clarify original and target pages during replace dialog #5369

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
Sereza7 wants to merge 2 commits into
base: master
Choose a base branch
from
+10 −7
Open

XWIKI-24144: Clarify original and target pages during replace dialog #5369

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d56f5f
XWIKI-24144: Clarify original and target pages during replace dialog
Sereza7 Apr 2, 2026
6ff4ece
XWIKI-24144: Clarify original and target pages during replace dialog
Sereza7 Apr 9, 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
2 changes: 2 additions & 0 deletions ...ctoring/xwiki-platform-refactoring-api/src/main/resources/ApplicationResources.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ job.log.label.refactoring/copy=Copy log
job.question.OverwriteQuestion.title=Replace page "{0}"?
job.question.OverwriteQuestion.message=An older page with the same name already exists in "{0}". Replacing it will overwrite its content.
job.question.OverwriteQuestion.destination=Original page
job.question.OverwriteQuestion.destination.hint=The content of this page will be overwritten and lost.
job.question.OverwriteQuestion.source=Replace with
job.question.OverwriteQuestion.source.hint=This page's content will be kept.
job.question.OverwriteQuestion.lastAuthor=Last author: {0}
job.question.OverwriteQuestion.lastModified=Last modified: {0}
job.question.OverwriteQuestion.applyToAll=Apply this action to all the following pages.
Expand Down
15 changes: 8 additions & 7 deletions ...iki-platform-web-templates/src/main/resources/templates/job/question/OverwriteQuestion.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@
#set ($destinationPath = '/')
#end

#macro (displayDocumentSummary $document $title)
#macro (displayDocumentSummary $document $titleKey)
#set($title = $escapetool.xml($services.localization.render("job.question.OverwriteQuestion.$!{titleKey}")))
#set($hint = $escapetool.xml($services.localization.render("job.question.OverwriteQuestion.$!{titleKey}.hint")))
<div class="col-sm-6">
<h5>
$services.icon.renderHTML('file-white')
<a href="$document.getURL()">$escapetool.xml($services.localization.render($title))</a>
</h5>
<label>$title</label>
<div class="xHint">$hint</div>
<a href="$document.getURL()">$services.icon.renderHTML('file-white')$document.plainTitle</a>
Copy link
Copy Markdown
Contributor

@manuelleduc manuelleduc Apr 3, 2026

Choose a reason for hiding this comment

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

If not too much work, I'd be curious to see the difference between

  • this
  • this but with a non-breakable space between the icon of the text
  • the icon outside the link

I'm also interested in what it looks like on hover. I think someone mentioned a possibly odd look with the underline spanning below the icon.

The rest of the PR looks ok, thanks for your work :)

Copy link
Copy Markdown
Contributor

@michitux michitux Apr 7, 2026

Choose a reason for hiding this comment

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

Printing $document.plainTitle without XML-escaping introduces an XSS vulnerability. This needs to be fixed (escaped).

Copy link
Copy Markdown
Contributor Author

@Sereza7 Sereza7 Apr 9, 2026

Choose a reason for hiding this comment

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

@manuelleduc

If not too much work, I'd be curious to see the difference between [...]

Here are screenshots of the three options in the same order, with the mouse hovering on one of the two links:
Screenshot From 2026-04-09 13-59-09
Screenshot From 2026-04-09 13-52-07
Screenshot From 2026-04-09 13-58-27

I decided to add the nbsp in 6ff4ece because I believe it's what we do in most places and it looks a bit better. This looks a bit odd when hovered but IMO it's alright.
The only strong opinion I have on this is that I think it makes sense to make the icon clickable so we should include it in the link.

I think someone mentioned a possibly odd look with the underline spanning below the icon.

Just to be sure, I checked with Silk and IMO things look okay here too:
Screenshot From 2026-04-09 14-02-23

I think this was a comment made about the underlining under "nothing" between the icon and the text. I remember a comment was done when we changed it in the notification dropdown:
image

This is an argument against adding a NBSP here, but in my opinion it's not strong enough to enforce the choice.

It's a small layout detail, but it could be something to include in the UX guidelines to share with Cristal @tkrieck has proposed on the forum. Something like:
When using an icon with its explaining text, separate them with a non breaking space. (vocab is probably too implementation centered)
or
When using an icon with its explaining text, do not separate them with any character.

@michitux

Printing $document.plainTitle without XML-escaping introduces an XSS vulnerability. This needs to be fixed (escaped).

Good find! Sorry for missing this one. I fixed it in 6ff4ece 👍

<div>
$services.localization.render('job.question.OverwriteQuestion.lastAuthor',
[$xwiki.getUserName($document.authorReference)])
Expand All @@ -70,8 +71,8 @@
$escapetool.xml($services.localization.render('job.question.OverwriteQuestion.message', [$destinationPath]))
</p>
<div class="row form-group">
#displayDocumentSummary($destinationDocument 'job.question.OverwriteQuestion.destination')
#displayDocumentSummary($sourceDocument 'job.question.OverwriteQuestion.source')
#displayDocumentSummary($destinationDocument 'destination')
#displayDocumentSummary($sourceDocument 'source')
</div>
<div>
<label>
Expand Down