Skip to content

show parameters of a run with readOnlyMode #10941

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
krisstern merged 2 commits into from
Aug 13, 2025
Merged

show parameters of a run with readOnlyMode #10941

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
a5bb0fd
show parameters readonly
mawinter69 Aug 8, 2025
00770c4
fix test
mawinter69 Aug 10, 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
3 changes: 2 additions & 1 deletion core/src/main/resources/hudson/model/BooleanParameterValue/value.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ THE SOFTWARE.
xmlns:i="jelly:fmt" xmlns:p="/lib/hudson/project">
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry description="${it.formattedDescription}">
<f:checkbox title="${h.escape(it.name)}" name="value" checked="${it.value}" readonly="true" />
<j:set var="readOnlyMode" value="true"/>
<f:checkbox title="${h.escape(it.name)}" name="value" checked="${it.value}"/>
</f:entry>
</j:jelly>
1 change: 1 addition & 0 deletions core/src/main/resources/hudson/model/ParametersAction/index.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ THE SOFTWARE.
<l:main-panel>
<t:buildCaption it="${build}">${title}</t:buildCaption>
<j:set var="escapeEntryTitleAndDescription" value="true" /> <!-- SECURITY-353 defense unless overridden -->
<j:set var="readOnlyMode" value="true"/>
<j:forEach var="parameterValue" items="${it.parameters}">
<st:include it="${parameterValue}" page="value.jelly" />
</j:forEach>
Expand Down
13 changes: 7 additions & 6 deletions core/src/main/resources/hudson/model/StringParameterValue/value.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ THE SOFTWARE.

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"
xmlns:i="jelly:fmt" xmlns:p="/lib/hudson/project">
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<f:textbox name="value" value="${it.value}" readonly="true" />
</f:entry>
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"
xmlns:i="jelly:fmt" xmlns:p="/lib/hudson/project">
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<j:set var="readOnlyMode" value="true"/>
<f:textbox name="value" value="${it.value}"/>
</f:entry>
</j:jelly>
9 changes: 5 additions & 4 deletions core/src/main/resources/hudson/model/TextParameterValue/value.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ THE SOFTWARE.
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"
xmlns:i="jelly:fmt" xmlns:p="/lib/hudson/project">
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<f:textarea name="value" value="${it.value}" readonly="readonly" />
</f:entry>
<j:set var="escapeEntryTitleAndDescription" value="false"/>
<f:entry title="${h.escape(it.name)}" description="${it.formattedDescription}">
<j:set var="readOnlyMode" value="true"/>
<f:textarea name="value" value="${it.value}"/>
</f:entry>
</j:jelly>
4 changes: 2 additions & 2 deletions core/src/main/resources/lib/form/checkbox.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ THE SOFTWARE.
name="${name}"
value="${attrs.value}"
title="${attrs.tooltip}"
onclick="${attrs.readonly=='true' ? 'return false;' : attrs.onclick}" id="${attrs.id}" class="${attrs.class} ${attrs.negative!=null ? 'negative' : null} ${attrs.checkUrl!=null?'validated':''}"
onclick="${attrs.readonly=='true' ? null : attrs.onclick}" id="${attrs.id}" class="${attrs.class} ${attrs.negative!=null ? 'negative' : null} ${attrs.checkUrl!=null?'validated':''}"
checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" json="${attrs.json}"
disabled="${readOnlyMode ? 'true' : null}"
disabled="${readOnlyMode or attrs.readonly=='true' ? 'true' : null}"
Copy link
Copy Markdown
Member

@daniel-beck daniel-beck Aug 13, 2025

Choose a reason for hiding this comment

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

#8420 (comment) points out this changes the semantics of form submission. Could this cause trouble?

CC @zbynek as comment author

Copy link
Copy Markdown
Contributor Author

@mawinter69 mawinter69 Aug 14, 2025

Choose a reason for hiding this comment

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

I checked where checkboxes with readonly="true" are used. I found 6 plugins:

  • artifact-manager-s3: 2 places the underlying fields are final and the value is set from a system property -> no problem
  • admin-params: used to show a parameter value in the parameters page of a run -> no problem
  • google-compute-egine: the underlying field is static final -> no problem
  • agent-maintenance: also sets readOnlyMode -> no problem
  • nodejs: used in show-config.jelly this only shows the config and should not be editable anyway -> no problem
  • matrix-combinations: used to show a parameter value in the parameters page of a run -> no problem

Copy link
Copy Markdown
Member

@timja timja Aug 14, 2025

Choose a reason for hiding this comment

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

Did you check with a screen reader?

Copy link
Copy Markdown
Contributor Author

@mawinter69 mawinter69 Aug 14, 2025

Choose a reason for hiding this comment

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

ahh yes that could be a problem.

But then any place where we set readOnlyMode that leads to a disabled input is problematic. E.g. in radio.jelly, radioBlock.jelly toggleSwitch.jelly, enum.jelly

checked="${value ? 'true' : null}"/>
<label class="attach-previous ${attrs.title == null ? 'js-checkbox-label-empty' : ''}"
title="${attrs.tooltip}">${attrs.title}</label>
Expand Down
10 changes: 8 additions & 2 deletions core/src/main/resources/lib/form/radio.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ THE SOFTWARE.

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define">
<st:documentation> <![CDATA[
<st:documentation> <![CDATA[
<input type="radio"> tag that takes true/false for @checked, which is more Jelly friendly.

Note that Safari doesn't support onchange.
Expand All @@ -38,7 +38,13 @@ THE SOFTWARE.
<st:attribute name="checked" />
<st:attribute name="value" />
<st:attribute name="id" />
<st:attribute name="onclick" />
<st:attribute name="onclick" deprecated="true">
Inline JavaScript to execute when the checkbox is clicked.
Deprecated because this attribute is incompatible with adding Content-Security-Policy to the Jenkins UI in the future.
Set 'id' or 'class' attributes as appropriate to look up this element in external Javascript files (e.g. adjuncts)
to add the desired behavior there (DOMContentLoaded event in static forms, Behaviour.specify if this element may be
dynamically added). See https://github.com/jenkinsci/jenkins/pull/6852 for an example.
</st:attribute>
<st:attribute name="title">
If specified, this human readable text will follow the radio, and clicking this text also
toggles the radio.
Expand Down
2 changes: 1 addition & 1 deletion test/src/test/java/hudson/model/ParametersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void xss() throws Exception {
assertAll(
() -> assertThat("parameters page should escape param name", text2, containsString("&lt;param name&gt;")),
() -> assertThat("parameters page should not leave param name unescaped", text2, not(containsString("<param name>"))),
() -> assertThat("parameters page should escape param value", text2, containsString("&lt;param value&gt;")),
() -> assertThat("parameters page should escape param value", text2, containsString("&lt;param value>")),
() -> assertThat("parameters page should not leave param value unescaped", text2, not(containsString("<param value>"))),
() -> assertThat("parameters page should mark up param description", text2, containsString("<b>[</b>param description<b>]</b>")),
() -> assertThat("parameters page should not leave param description unescaped", text2, not(containsString("<param description>")))
Expand Down
Loading