diff --git a/changelog.html b/changelog.html index f1ea624a8b4b1146cf7586ec69ca782a31cf1e71..2e42b3cb89a1d77879d4c0dfa16b43fc48daf503 100644 --- a/changelog.html +++ b/changelog.html @@ -58,6 +58,8 @@ Upcoming changes
  • Allow blank rootDN in LDAPSecurityRealm. (thread) +
  • + Fixed the UI rendering problem when certain controls are nested together.
  • Add active configurations in remote API for matrix projects. (issue 9248) diff --git a/core/src/main/resources/lib/form/dropdownListBlock.jelly b/core/src/main/resources/lib/form/dropdownListBlock.jelly index 103e5624ef3328a1d0dfa2f2b49e188f8cafaa70..d06278a6734f59178040be5beaa0a533d7ba6159 100644 --- a/core/src/main/resources/lib/form/dropdownListBlock.jelly +++ b/core/src/main/resources/lib/form/dropdownListBlock.jelly @@ -50,9 +50,9 @@ THE SOFTWARE. - + - + @@ -65,7 +65,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/lib/form/optionalBlock.jelly b/core/src/main/resources/lib/form/optionalBlock.jelly index 87ce8bbd3732fb010016e2150476cd3e84f878e0..b0020f32ed848be16d7346f74ba8dd8b72405102 100644 --- a/core/src/main/resources/lib/form/optionalBlock.jelly +++ b/core/src/main/resources/lib/form/optionalBlock.jelly @@ -80,9 +80,10 @@ THE SOFTWARE. + - + diff --git a/test/src/test/java/lib/form/RowVisibilityGroupTest.java b/test/src/test/java/lib/form/RowVisibilityGroupTest.java new file mode 100644 index 0000000000000000000000000000000000000000..81be756e08918566f226582e98dd90e01dae1ee3 --- /dev/null +++ b/test/src/test/java/lib/form/RowVisibilityGroupTest.java @@ -0,0 +1,172 @@ +package lib.form; + +import com.gargoylesoftware.htmlunit.html.HtmlElement; +import com.gargoylesoftware.htmlunit.html.HtmlInput; +import com.gargoylesoftware.htmlunit.html.HtmlOption; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.html.HtmlSelect; +import hudson.model.AbstractDescribableImpl; +import hudson.model.Describable; +import hudson.model.Descriptor; +import net.sf.json.JSONObject; +import org.jvnet.hudson.test.HudsonTestCase; +import org.jvnet.hudson.test.TestExtension; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.StaplerRequest; + +import java.util.List; + +/** + * Tests the 'rowvg-start' and 'rowvg-end' CSS attributes and their effects. + * + *

    + * Some of our tags, such as <optionalBlock> and <dropdownList> involves grouping of sibling table rows, + * and controlling visibility of them. So when such tags nest to each other, the visibility updates need to be + * done carefully, or else the visibility could get out of sync with the model (imagine outer group is made visible + * while inner group is not visible --- if all the rows are simply enumerated and visibility changed, we end up + * making the inner group visible.) + * + *

    + * The rowVisibilityGroup object in hudson-behavior.js is used to coordinate this activity, and this test + * ensures that it's working. + * + * @author Kohsuke Kawaguchi + */ +public class RowVisibilityGroupTest extends HudsonTestCase implements Describable { + public Drink drink; + private Beer beer; + + /** + * Nested optional blocks + */ + public void test1() throws Exception { + HtmlPage p = createWebClient().goTo("self/test1"); + + HtmlElement outer = (HtmlElement)p.selectSingleNode("//INPUT[@name='outer']"); + HtmlElement inner = (HtmlElement)p.selectSingleNode("//INPUT[@name='inner']"); + HtmlInput field = (HtmlInput)p.selectSingleNode("//INPUT[@type='text'][@name='_.field']"); + + // outer gets unfolded, but inner should be still folded + outer.click(); + assertFalse(field.isDisplayed()); + // now click inner, to reveal the field + inner.click(); + assertTrue(field.isDisplayed()); + + // folding outer should hide everything + outer.click(); + assertFalse(field.isDisplayed()); + // but if we unfold outer, everything should be revealed because inner is already checked. + outer.click(); + assertTrue(field.isDisplayed()); + } + + /** + * optional block inside the dropdownDescriptorSelector + */ + public void test2() throws Exception { + HtmlPage p = createWebClient().goTo("self/test2"); + + HtmlSelect s = (HtmlSelect)p.selectSingleNode("//SELECT"); + List opts = s.getOptions(); + + // those first selections will load additional HTMLs + s.setSelectedAttribute(opts.get(0),true); + s.setSelectedAttribute(opts.get(1),true); + + // now select back what's already loaded, to cause the existing elements to be displayed + s.setSelectedAttribute(opts.get(0),true); + + // make sure that the inner control is still hidden + List textboxes = p.selectNodes("//INPUT[@name='_.textbox2']"); + assertEquals(2,textboxes.size()); + for (HtmlInput e : textboxes) + assertTrue(!e.isDisplayed()); + + // reveal the text box + List checkboxes = p.selectNodes("//INPUT[@name='inner']"); + assertEquals(2,checkboxes.size()); + checkboxes.get(0).click(); + assertTrue(textboxes.get(0).isDisplayed()); + textboxes.get(0).type("Budweiser"); + + // toggle the selection again + s.setSelectedAttribute(opts.get(1),true); + s.setSelectedAttribute(opts.get(0),true); + + // make sure it's still displayed this time + assertTrue(checkboxes.get(0).isChecked()); + assertTrue(textboxes.get(0).isDisplayed()); + + // make sure we get what we expect + submit(p.getFormByName("config")); + assertEqualDataBoundBeans(beer,new Beer("",new Nested("Budweiser"))); + } + + public void doSubmitTest2(StaplerRequest req) throws Exception { + JSONObject json = req.getSubmittedForm(); + System.out.println(json); + beer = (Beer)req.bindJSON(Drink.class,json.getJSONObject("drink")); + } + + public DescriptorImpl getDescriptor() { + return hudson.getDescriptorByType(DescriptorImpl.class); + } + + @TestExtension + public static final class DescriptorImpl extends Descriptor { + public String getDisplayName() { + return null; + } + } + + public static class Nested { + public String textbox2; + + @DataBoundConstructor + public Nested(String textbox2) { + this.textbox2 = textbox2; + } + } + + public static abstract class Drink extends AbstractDescribableImpl { + public String textbox1; + public Nested inner; + + @DataBoundConstructor + protected Drink(String textbox1, Nested inner) { + this.textbox1 = textbox1; + this.inner = inner; + } + } + + public static class Beer extends Drink { + @DataBoundConstructor + public Beer(String textbox1, Nested inner) { + super(textbox1, inner); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor { + @Override + public String getDisplayName() { + return "Beer"; + } + } + } + + public static class Coke extends Drink { + @DataBoundConstructor + public Coke(String textbox1, Nested inner) { + super(textbox1, inner); + } + + @TestExtension + public static class DescriptorImpl extends Descriptor { + @Override + public String getDisplayName() { + return "Coke"; + } + } + } +} diff --git a/test/src/test/resources/lib/form/RowVisibilityGroupTest/Drink/config.jelly b/test/src/test/resources/lib/form/RowVisibilityGroupTest/Drink/config.jelly new file mode 100644 index 0000000000000000000000000000000000000000..876bf13c6fe943249cf9083eac8526ee253d6927 --- /dev/null +++ b/test/src/test/resources/lib/form/RowVisibilityGroupTest/Drink/config.jelly @@ -0,0 +1,35 @@ + + + + + + + + + + + + + diff --git a/test/src/test/resources/lib/form/RowVisibilityGroupTest/test1.jelly b/test/src/test/resources/lib/form/RowVisibilityGroupTest/test1.jelly new file mode 100644 index 0000000000000000000000000000000000000000..0fa2e8d3942c59a52467c9c180cc59f062185552 --- /dev/null +++ b/test/src/test/resources/lib/form/RowVisibilityGroupTest/test1.jelly @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + + + + diff --git a/test/src/test/resources/lib/form/RowVisibilityGroupTest/test2.jelly b/test/src/test/resources/lib/form/RowVisibilityGroupTest/test2.jelly new file mode 100644 index 0000000000000000000000000000000000000000..fdc50a2b7337f56f07713278c7450e1481a9838e --- /dev/null +++ b/test/src/test/resources/lib/form/RowVisibilityGroupTest/test2.jelly @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + diff --git a/war/src/main/webapp/scripts/behavior.js b/war/src/main/webapp/scripts/behavior.js index ae97aad6223651d2775d0dbd2e98cec2f91b27f3..d1c3947d029ee3f7e986b226fbcf0dfa8734c43f 100644 --- a/war/src/main/webapp/scripts/behavior.js +++ b/war/src/main/webapp/scripts/behavior.js @@ -54,12 +54,30 @@ var Behaviour = { this.applySubtree(document); }, + /** + * Applies behaviour rules to a subtree/subforest. + * + * @param {HTMLElement|HTMLElement[]} startNode + * Subtree/forest to apply rules. + * + * Within a single subtree, rules are the outer loop and the nodes in the tree are the inner loop, + * and sometimes the behaviour rules rely on this ordering to work correctly. When you pass a forest, + * this semantics is preserved. + */ applySubtree : function(startNode,includeSelf) { Behaviour.list._each(function(sheet) { for (var selector in sheet){ - var list = findElementsBySelector(startNode,selector,includeSelf); - if (list.length>0) // just to simplify setting of a breakpoint. - list._each(sheet[selector]); + function apply(n) { + var list = findElementsBySelector(n,selector,includeSelf); + if (list.length>0) // just to simplify setting of a breakpoint. + list._each(sheet[selector]); + } + + if (startNode instanceof Array) { + startNode._each(apply) + } else { + apply(startNode); + } } }); }, diff --git a/war/src/main/webapp/scripts/hudson-behavior.js b/war/src/main/webapp/scripts/hudson-behavior.js index cbc78944d4a847467e008c2a383adc31b376f3ec..4c2772ba495d9f3ed28959e85d5b2852ef51ca1c 100644 --- a/war/src/main/webapp/scripts/hudson-behavior.js +++ b/war/src/main/webapp/scripts/hudson-behavior.js @@ -240,7 +240,7 @@ function findFollowingTR(input, className) { // then next TR that matches the CSS do { tr = tr.nextSibling; - } while (tr != null && (tr.tagName != "TR" || tr.className != className)); + } while (tr != null && (tr.tagName != "TR" || !Element.hasClassName(tr,className))); return tr; } @@ -453,8 +453,7 @@ function renderOnDemand(e,callback,noBehaviour) { } Element.remove(e); - t.responseText.evalScripts(); - elements.each(function(n) { Behaviour.applySubtree(n,true); }); + Behaviour.applySubtree(elements,true); if (callback) callback(t); }); @@ -805,6 +804,81 @@ var hudsonRules = { e.setAttribute("ref", checkbox.id = "cb"+(iota++)); }, + // see RowVisibilityGroupTest + "TR.rowvg-start" : function(e) { + // figure out the corresponding end marker + function findEnd(e) { + for( var depth=0; ; e=e.nextSibling) { + if(Element.hasClassName(e,"rowvg-start")) depth++; + if(Element.hasClassName(e,"rowvg-end")) depth--; + if(depth==0) return e; + } + } + + e.rowVisibilityGroup = { + outerVisible: true, + innerVisible: true, + /** + * TR that marks the beginning of this visibility group. + */ + start: e, + /** + * TR that marks the end of this visibility group. + */ + end: findEnd(e), + + /** + * Considers the visibility of the row group from the point of view of outside. + * If you think of a row group like a logical DOM node, this is akin to its .style.display. + */ + makeOuterVisisble : function(b) { + this.outerVisible = b; + this.updateVisibility(); + }, + + /** + * Considers the visibility of the rows in this row group. Since all the rows in a rowvg + * shares the single visibility, this just needs to be one boolean, as opposed to many. + * + * If you think of a row group like a logical DOM node, this is akin to its children's .style.display. + */ + makeInnerVisisble : function(b) { + this.innerVisible = b; + this.updateVisibility(); + }, + + /** + * Based on innerVisible and outerVisible, update the relevant rows' actual CSS display attribute. + */ + updateVisibility : function() { + var display = (this.outerVisible && this.innerVisible) ? "" : "none"; + for (var e=this.start; e!=this.end; e=e.nextSibling) { + if (e.rowVisibilityGroup && e!=this.start) { + e.rowVisibilityGroup.makeOuterVisisble(this.innerVisible); + e = e.rowVisibilityGroup.end; // the above call updates visibility up to e.rowVisibilityGroup.end inclusive + } else { + e.style.display = display; + } + } + }, + + /** + * Enumerate each row and pass that to the given function. + * + * @param {boolean} recursive + * If true, this visits all the rows from nested visibility groups. + */ + eachRow : function(recursive,f) { + if (recursive) { + for (var e=this.start; e!=this.end; e=e.nextSibling) + f(e); + } else { + throw "not implemented yet"; + } + } + }; + }, + "TR.row-set-end": function(e) { // see rowSet.jelly and optionalBlock.jelly // figure out the corresponding start block var end = e; @@ -946,20 +1020,39 @@ var hudsonRules = { "SELECT.dropdownList" : function(e) { if(isInsideRemovable(e)) return; - e.subForms = []; + var subForms = []; var start = findFollowingTR(e, 'dropdownList-container').firstChild.nextSibling, end; do { start = start.firstChild; } while (start && start.tagName != 'TR'); - if (start && start.className != 'dropdownList-start') + + if (start && !Element.hasClassName(start,'dropdownList-start')) start = findFollowingTR(start, 'dropdownList-start'); while (start != null) { - end = findFollowingTR(start, 'dropdownList-end'); - e.subForms.push({ 'start': start, 'end': end }); - start = findFollowingTR(end, 'dropdownList-start'); + subForms.push(start); + start = findFollowingTR(start, 'dropdownList-start'); + } + + // control visibility + function updateDropDownList() { + for (var i = 0; i < subForms.length; i++) { + var show = e.selectedIndex == i; + var f = subForms[i]; + + if (show) renderOnDemand(f.nextSibling); + f.rowVisibilityGroup.makeInnerVisisble(show); + + // TODO: this is actually incorrect in the general case if nested vg uses field-disabled + // so far dropdownList doesn't create such a situation. + f.rowVisibilityGroup.eachRow(true, show?function(e) { + e.removeAttribute("field-disabled"); + } : function(e) { + e.setAttribute("field-disabled","true"); + }); + } } - e.onchange = function () { updateDropDownList(e); }; + e.onchange = updateDropDownList; - updateDropDownList(e); + updateDropDownList(); }, // select.jelly @@ -1117,6 +1210,7 @@ function applyNameRef(s,e,id) { } } + // used by optionalBlock.jelly to update the form status // @param c checkbox element function updateOptionalBlock(c,scroll) { @@ -1124,39 +1218,21 @@ function updateOptionalBlock(c,scroll) { var s = c; while(!Element.hasClassName(s, "optional-block-start")) s = s.parentNode; - var tbl = s.parentNode; - var i = false; - var o = false; - - var checked = xor(c.checked,Element.hasClassName(c,"negative")); - var lastRow = null; - - for (var j = 0; tbl.rows[j]; j++) { - var n = tbl.rows[j]; - if (i && Element.hasClassName(n, "optional-block-end")) - o = true; + // find the beginning of the rowvg + var vg = s; + while (!Element.hasClassName(vg,"rowvg-start")) + vg = vg.nextSibling; - if (i && !o) { - if (checked) { - n.style.display = ""; - lastRow = n; - } else - n.style.display = "none"; - } + var checked = xor(c.checked,Element.hasClassName(c,"negative")); - if (n==s) { - if (n.getAttribute('hasHelp') == 'true') - j++; - i = true; - } - } + vg.rowVisibilityGroup.makeInnerVisisble(checked); if(checked && scroll) { var D = YAHOO.util.Dom; var r = D.getRegion(s); - if(lastRow!=null) r = r.union(D.getRegion(lastRow)); + r = r.union(D.getRegion(vg.rowVisibilityGroup.end)); scrollIntoView(r); } @@ -1361,26 +1437,6 @@ Form.findMatchingInput = function(base, name) { return null; // not found } -// used witih and to control visibility -function updateDropDownList(sel) { - for (var i = 0; i < sel.subForms.length; i++) { - var show = sel.selectedIndex == i; - var f = sel.subForms[i]; - var tr = f.start; - while (true) { - tr.style.display = (show ? "" : "none"); - if(show) { - tr.removeAttribute("field-disabled"); - renderOnDemand(tr); - } else // buildFormData uses this attribute and ignores the contents - tr.setAttribute("field-disabled","true"); - if (tr == f.end) break; - tr = tr.nextSibling; - } - } -} - - // code for supporting repeatable.jelly var repeatableSupport = { // set by the inherited instance to the insertion point DIV