Skip to content
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.

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

Inline form element path #5926

Merged
merged 10 commits into from
Nov 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package jenkins.formelementpath;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.Main;
import hudson.model.PageDecorator;
import jenkins.util.SystemProperties;

@Extension
public class FormElementPathPageDecorator extends PageDecorator {

@SuppressFBWarnings("MS_SHOULD_BE_FINAL")
private static /*almost final */ boolean ENABLED = Main.isUnitTest ||
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should still be able to flip this switch via the installation of a plugin?
Currently we spin up some Jenkins' instance where we do not configure system properties (or doing so is hard) and the current flow is to install the form-element-path plugin without the use of the paths it provides. (e.g. auto provisioning in kubernetes)

@Vlatombe knows more about this area of the framework that we use internally.

Copy link
Member

Choose a reason for hiding this comment

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

That's fine, we'll be able to provide the system property.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could also make it public as well

Copy link
Member

Choose a reason for hiding this comment

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

as Vincent is ok with being able to provide it, we can keep it as it is for now (easier to open up, than restrict accesses later)

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to turn this off by default, even in tests, at least until it can be rewritten to not rely on DOM listeners: jenkinsci/bom#746 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah that’s a shames :(

Copy link
Member

Choose a reason for hiding this comment

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

#6004

SystemProperties.getBoolean(FormElementPathPageDecorator.class.getName() + ".enabled");
Copy link
Member

Choose a reason for hiding this comment

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

possible future enhancement would be to also set this for RealJenkinsRule

Copy link
Member

Choose a reason for hiding this comment

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

That should set isUnitTest IIRC.


public boolean isEnabled() {
return ENABLED;
}

timja marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler">
<j:if test="${it.enabled}">
<st:adjunct includes="jenkins.formelementpath.form-element-path"/>
</j:if>
</j:jelly>
196 changes: 196 additions & 0 deletions core/src/main/resources/jenkins/formelementpath/form-element-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
/**
* Adds a 'path' attribute to form elements in the DOM.
* This is useful for providing stable selectors for UI testing.
*
* Instead of selecting by xpath with something like div/span/input[text() = 'Name']
* You can use the path attribute: /org-jenkinsci-plugins-workflow-libs-FolderLibraries/libraries/name
*/
document.addEventListener("DOMContentLoaded", function(){
// most of this is copied from hudson-behaviour.js
function buildFormTree(form) {
form.formDom = {}; // root object

var doms = []; // DOMs that we added 'formDom' for.
doms.push(form);

function addProperty(parent, name, value) {
name = shortenName(name);
if (parent[name] != null) {
if (parent[name].push == null) // is this array?
parent[name] = [parent[name]];
parent[name].push(value);
} else {
parent[name] = value;
}
}

// find the grouping parent node, which will have @name.
// then return the corresponding object in the map
function findParent(e) {
var p = findFormParent(e, form);
if (p == null) return {};

var m = p.formDom;
if (m == null) {
// this is a new grouping node
doms.push(p);
p.formDom = m = {};
addProperty(findParent(p), p.getAttribute("name"), p);
}
return m;
}

var jsonElement = null;

for (var i = 0; i < form.elements.length; i++) {
var e = form.elements[i];
if (e.name == "json") {
jsonElement = e;
continue;
}
if (e.tagName == "FIELDSET")
continue;
if (e.tagName == "SELECT" && e.multiple) {
addProperty(findParent(e), e.name, e);
continue;
}

var p;
var type = e.getAttribute("type");
if (type == null) type = "";
switch (type.toLowerCase()) {
case "button":
var element
// modern buttons aren't wrapped in spans
if (e.classList.contains('jenkins-button')) {
element = e
} else {
p = findParent(e);
element = e.parentNode.parentNode; // YUI's surrounding <SPAN> that has interesting classes
}
var name = null;
["repeatable-add", "repeatable-delete", "hetero-list-add", "expand-button", "advanced-button", "apply-button", "validate-button"]
.forEach(function (clazz) {
if (element.classList.contains(clazz)) {
name = clazz;
}
});
if (name == null) {
if (name == null) {
element = element.parentNode.previousSibling;
if (element != null && element.classList && element.classList.contains('repeatable-insertion-point')) {
name = "hetero-list-add";
}
}
}
if (name != null) {
addProperty(p, name, e);
}
break;
case "submit":
break;
case "checkbox":
case "radio":
p = findParent(e);
if (e.groupingNode) {
e.formDom = {};
}
addProperty(p, e.name, e);
break;
case "file":
// to support structured form submission with file uploads,
// rename form field names to unique ones, and leave this name mapping information
// in JSON. this behavior is backward incompatible, so only do
// this when
p = findParent(e);
if (e.getAttribute("jsonAware") != null) {
var on = e.getAttribute("originalName");
if (on != null) {
addProperty(p, on, e);
} else {
addProperty(p, e.name, e);
}
}
break;
// otherwise fall through
default:
p = findParent(e);
addProperty(p, e.name, e);
break;
}
}

function annotate(e, path) {
e.setAttribute("path", path);
var o = e.formDom || {};
for (var key in o) {
var v = o[key];

function child(v, i) {
var suffix = null;
var newKey = key;
if (v.parentNode.className && v.parentNode.className.indexOf("one-each") > -1 && v.parentNode.className.indexOf("honor-order") > -1) {
suffix = v.getAttribute("descriptorId").split(".").pop()
} else if (v.getAttribute("type") == "radio") {
suffix = v.value
while (newKey.substring(0, 8) == 'removeme')
newKey = newKey.substring(newKey.indexOf('_', 8) + 1);
} else if (v.getAttribute("suffix") != null) {
suffix = v.getAttribute("suffix")
} else {
if (i > 0)
suffix = i;
}
if (suffix == null) suffix = "";
else suffix = '[' + suffix + ']';

annotate(v, path + "/" + newKey + suffix);
}

if (v instanceof Array) {
var i = 0;
v.forEach(function (v) {
child(v, i++)
})
} else {
child(v, 0)
}
}

}

annotate(form, "");

// clean up
for (i = 0; i < doms.length; i++)
doms[i].formDom = null;

return true;
}

function applyAll() {
document.querySelectorAll("FORM").forEach(function (e) {
buildFormTree(e);
})
}

/* JavaScript sometimes re-arranges the DOM and doesn't call layout callback
* known cases: YUI buttons, CodeMirror.
Copy link
Member

Choose a reason for hiding this comment

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

Ahhhh!!!! I wonder if this may well explain some issues we have with codemirror sections not being found in the ATH!

* We run apply twice to work around this, once immediately so that most cases work and the tests don't need to wait,
* and once to catch the edge cases.
*/
function hardenedApplyAll () {
applyAll();

setTimeout(function () {
applyAll();
}, 1000);
}

hardenedApplyAll();

layoutUpdateCallback.add(hardenedApplyAll)

// expose this globally so that Selenium can call it
window.recomputeFormElementPath = hardenedApplyAll;
});
7 changes: 5 additions & 2 deletions core/src/main/resources/lib/form/repeatable/repeatable.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,10 @@ var repeatableSupport = {
a.onComplete.subscribe(function() {
var p = n.parentNode;
p.removeChild(n);
if (p.tag)
if (p.tag) {
p.tag.update();
}
layoutUpdateCallback.call();
});
a.animate();
},
Expand All @@ -145,6 +147,7 @@ var repeatableSupport = {
updateOptionalBlock(input, false);
}
}
layoutUpdateCallback.call();
}
};

Expand Down Expand Up @@ -185,7 +188,7 @@ Behaviour.specify("INPUT.repeatable-delete", 'repeatable', 0, function(e) {
e = be = null; // avoid memory leak
});

// radio buttons in repeatable content
// radio buttons in repeatable content
// Needs to run before the radioBlock behavior so that names are already unique.
Behaviour.specify("DIV.repeated-chunk", 'repeatable', -200, function(d) {
var inputs = d.getElementsByTagName('INPUT');
Expand Down