Skip to content

[FIX JENKINS-54325] Add support for multiple SimplePageDecorators in … #3710

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

Closed
jvz wants to merge 4 commits into from
Closed

[FIX JENKINS-54325] Add support for multiple SimplePageDecorators in … #3710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3fd970c
[FIX JENKINS-54325] Add support for multiple SimplePageDecorators in …
jvz Oct 29, 2018
89ccd6a
Deprecate Functions.getSimplePageDecorator
jvz Oct 30, 2018
245f979
Add id to login password field
jvz Oct 30, 2018
213ec32
Add support for multiple SimplePageDecorators in signup.jelly
jvz Oct 30, 2018
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
10 changes: 10 additions & 0 deletions core/src/main/java/hudson/Functions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1774,11 +1774,21 @@ public static List<PageDecorator> getPageDecorators() {
/**
* Gets only one {@link SimplePageDecorator}.
* @since 2.128
* @deprecated use {@link #getSimplePageDecorators()} instead
*/
@Deprecated
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

Nupp

public static SimplePageDecorator getSimplePageDecorator() {
return SimplePageDecorator.first();
}

/**
* Gets all {@link SimplePageDecorator}s for the login page.
* @since TODO
*/
public static List<SimplePageDecorator> getSimplePageDecorators() {
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

👍

return SimplePageDecorator.all();
}

public static List<Descriptor<Cloud>> getCloudDescriptors() {
return Cloud.all();
}
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/jenkins/model/SimplePageDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
import hudson.ExtensionPoint;
import hudson.model.Describable;
import hudson.model.Descriptor;

import java.util.List;

/**
* Participates in the rendering of the login page
*
Expand Down Expand Up @@ -69,4 +72,12 @@ public static SimplePageDecorator first(){
}
}

/**
* Returns all login page decorators.
* @since TODO
*/
public static List<SimplePageDecorator> all() {
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

👍

return Jenkins.get().getDescriptorList(SimplePageDecorator.class);
}

}
12 changes: 9 additions & 3 deletions core/src/main/resources/hudson/security/HudsonPrivateSecurityRealm/signup.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ THE SOFTWARE.
<x:doctype name="html"/>
<!-- in case of error we want to surround the form elements with an error hint -->
<j:set var="inputClass" value="${data.errorMessage!=null ? 'danger' : 'normal'}"/>
<j:set var="simpleDecorator" value="${h.simplePageDecorator}"/>
<j:set var="simpleDecorators" value="${h.simplePageDecorators}"/>
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

You need both

<html lang="${request.getLocale().toLanguageTag()}">
<head data-rooturl="${rootURL}" data-resurl="${resURL}" resURL="${resURL}">
<title>${%Create an account! [Jenkins]}</title>
<!-- we do not want bots on this page -->
<meta name="ROBOTS" content="NOFOLLOW"/>
<!-- css styling, will fallback to default implementation -->
<st:include it="${simpleDecorator}" page="simple-head.jelly" optional="true"/>
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

Nupp

<j:forEach var="simpleDecorator" items="${simpleDecorators}">
<st:include it="${simpleDecorator}" page="simple-head.jelly" optional="true"/>
</j:forEach>
<link rel="stylesheet" href="${resURL}/css/signup.css" type="text/css"/>
</head>
<body>
Expand Down Expand Up @@ -292,6 +293,11 @@ THE SOFTWARE.
}
</script>
</j:if>
<div class="footer">
Copy link
Member Author

@jvz jvz Oct 30, 2018

Choose a reason for hiding this comment

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

I'm not so sure about the HTML structure here. My use case in paranoia-plugin is to insert a <script> element here, so the rendering doesn't matter too much. The default SimplePageDescriptor does not specify a footer fragment.

Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

I am fine with that

<j:forEach var="simpleDecorator" items="${simpleDecorators}">
<st:include it="${simpleDecorator}" page="simple-footer.jelly" optional="true"/>
</j:forEach>
</div>
</form>
</div>
</div>
Expand Down
16 changes: 11 additions & 5 deletions core/src/main/resources/jenkins/model/Jenkins/login.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,17 @@ THE SOFTWARE.
<j:set var="from" value="${error ? request.session.getAttribute('from') : request.getParameter('from')}"/>
<!-- in case of error we want to surround the form elements with an error hint -->
<j:set var="inputClass" value="${error ? 'danger' : 'normal'}"/>
<j:set var="simpleDecorator" value="${h.simplePageDecorator}"/>
<j:set var="simpleDecorators" value="${h.simplePageDecorators}"/>
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

see above, you need both

<!-- real deal starting here -->
<html lang="${request.getLocale().toLanguageTag()}">
<head data-rooturl="${rootURL}" data-resurl="${resURL}" resURL="${resURL}">
<title>${%signIn} [Jenkins]</title>
<!-- we do not want bots on this page -->
<meta name="ROBOTS" content="NOFOLLOW" />
<!-- css styling, will fallback to default implementation -->
<st:include it="${simpleDecorator}" page="simple-head.jelly" optional="true"/>
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

👎

<j:forEach var="simpleDecorator" items="${simpleDecorators}">
<st:include it="${simpleDecorator}" page="simple-head.jelly" optional="true"/>
</j:forEach>
</head>
<body>
<j:choose>
Expand All @@ -67,7 +69,9 @@ THE SOFTWARE.
<j:otherwise>
<div class="simple-page" role="main">
<div class="modal login">
<st:include it="${simpleDecorator}" page="simple-header.jelly" optional="true"/>
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

👎

<j:forEach var="simpleDecorator" items="${simpleDecorators}">
<st:include it="${simpleDecorator}" page="simple-header.jelly" optional="true"/>
</j:forEach>
<!-- login form -->
<form name="login" action="${it.securityRealm.authenticationGatewayUrl}" method="post">
<j:if test="${it.securityRealm.allowsSignup()}">
Expand Down Expand Up @@ -96,6 +100,7 @@ THE SOFTWARE.
class="${inputClass}"
type="password"
name="j_password"
id="j_password"
placeholder="${%Password}"
/>
</div>
Expand Down Expand Up @@ -145,8 +150,9 @@ THE SOFTWARE.
</j:forEach>

<div class="footer">

<st:include it="${simpleDecorator}" page="simple-footer.jelly" optional="true"/>
<j:forEach var="simpleDecorator" items="${simpleDecorators}">
Copy link
Contributor

@scherler scherler Nov 7, 2018

Choose a reason for hiding this comment

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

This is actually the only change you need besides creating the methods to get all decorators.

<st:include it="${simpleDecorator}" page="simple-footer.jelly" optional="true"/>
</j:forEach>
</div>

</div>
Expand Down