Struts 2
  1. Struts 2
  2. WW-3144

Label tag doesn't use "key" attribute properly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.6
    • Fix Version/s: 2.3.7
    • Component/s: None
    • Labels:
      None

      Description

      <s:label name="foo"/> works
      <s:label name="foo" label="%

      {getText('foo')}

      "/> works
      <s:label key="foo"/> puts the foo msg into the not-label label.

      Positively labelous.

      1. WW-3144.patch
        3 kB
        Lukasz Lenart

        Issue Links

          Activity

          Hide
          Wes Wannemacher added a comment -

          I'm not going to mess with this. I'll get 2.1.7 out and then we can work on this one.

          Show
          Wes Wannemacher added a comment - I'm not going to mess with this. I'll get 2.1.7 out and then we can work on this one.
          Hide
          musachy added a comment -

          assuming "labelKey" is set to "baz":

          <s:label key="labelKey"/>

          outputs:

          <label id="labelKey">baz</label>

          isn't that what it is supposed to do? I am confused

          Show
          musachy added a comment - assuming "labelKey" is set to "baz": <s:label key="labelKey"/> outputs: <label id="labelKey">baz</label> isn't that what it is supposed to do? I am confused
          Hide
          Dave Newton added a comment -

          Really? It was putting the msg under the key instead of the property for me.

          At least that's what I think I meant. In any case, I'll look at it--I was probably just not paying attention.

          Show
          Dave Newton added a comment - Really? It was putting the msg under the key instead of the property for me. At least that's what I think I meant. In any case, I'll look at it--I was probably just not paying attention.
          Hide
          musachy added a comment -

          I added this to LabelTest (test fails..i just wanted to see the output):

          public void testWithJustKeyValueFromStack() throws Exception

          { TestAction testAction = (TestAction) action; final String key = "labelKey"; final String value = "baz"; testAction.setText(key, value); LabelTag tag = new LabelTag(); tag.setTheme("simple"); tag.setPageContext(pageContext); tag.setKey(key); tag.doStartTag(); tag.doEndTag(); verify(LabelTest.class.getResource("Label-4.txt")); }

          and the output was:

          <label id="labelKey">baz</label>

          Show
          musachy added a comment - I added this to LabelTest (test fails..i just wanted to see the output): public void testWithJustKeyValueFromStack() throws Exception { TestAction testAction = (TestAction) action; final String key = "labelKey"; final String value = "baz"; testAction.setText(key, value); LabelTag tag = new LabelTag(); tag.setTheme("simple"); tag.setPageContext(pageContext); tag.setKey(key); tag.doStartTag(); tag.doEndTag(); verify(LabelTest.class.getResource("Label-4.txt")); } and the output was: <label id="labelKey">baz</label>
          Hide
          Wes Wannemacher added a comment -

          Dave, did you get a chance to look closer at this? I am going to bump it because I'm not exactly sure what's going on and I want to get moving on 2.1.8

          Show
          Wes Wannemacher added a comment - Dave, did you get a chance to look closer at this? I am going to bump it because I'm not exactly sure what's going on and I want to get moving on 2.1.8
          Hide
          Dave Newton added a comment -

          No; been out of commission for a couple of weeks w/ family medical issue. Heading home today, and once I fix the build on Macs will check it out.

          Show
          Dave Newton added a comment - No; been out of commission for a couple of weeks w/ family medical issue. Heading home today, and once I fix the build on Macs will check it out.
          Hide
          Lukasz Lenart added a comment -

          Dave, any progress with this issue?

          Show
          Lukasz Lenart added a comment - Dave, any progress with this issue?
          Hide
          Lukasz Lenart added a comment -

          Waiting for patch ...

          Show
          Lukasz Lenart added a comment - Waiting for patch ...
          Hide
          Lukasz Lenart added a comment -

          I'm not sure if I understood the problem right, but IMHO label tag shouldn't be wrapped with additional label for xhtml and css_xhtml theme - then there is a label for label

          Anyway, with simple theme, I've done something like this:

              protected void evaluateExtraParams() {
                  super.evaluateExtraParams();
          
                  // try value, then key, then name (this overrides the default behavior in the superclass)
                  if (value != null) {
                      addParameter("nameValue", findString(value));
                  } else if (key != null) {
                      Object nameValue = parameters.get("nameValue");
                      if (nameValue == null || nameValue.toString().length() == 0) {
                          // get the label from a TextProvider (default value is the key)
                          String providedLabel = TextProviderHelper.getText(key, key, stack);
                          addParameter("nameValue", providedLabel);
                      }
                  } else if (name != null) {
                      String expr = completeExpressionIfAltSyntax(name);
                      addParameter("nameValue", findString(expr));
                  }
          
                  if (forAttr != null) {
                      addParameter("for", findString(forAttr));
                  } else {
                      addParameter("for", getParameters().get("nameValue"));
                  }
              }
          

          and now the result is like this:

          <label id="labelKey" for="baz">baz</label>
          

          The id attribute wasn't translated which is ok, label should have a different Id than the "for" element of label, eg.:

          <label id="labelKey" for="baz">baz</label><input id="baz" value="foo"/>
          

          is it ok ?

          Show
          Lukasz Lenart added a comment - I'm not sure if I understood the problem right, but IMHO label tag shouldn't be wrapped with additional label for xhtml and css_xhtml theme - then there is a label for label Anyway, with simple theme, I've done something like this: protected void evaluateExtraParams() { super .evaluateExtraParams(); // try value, then key, then name ( this overrides the default behavior in the superclass) if (value != null ) { addParameter( "nameValue" , findString(value)); } else if (key != null ) { Object nameValue = parameters.get( "nameValue" ); if (nameValue == null || nameValue.toString().length() == 0) { // get the label from a TextProvider ( default value is the key) String providedLabel = TextProviderHelper.getText(key, key, stack); addParameter( "nameValue" , providedLabel); } } else if (name != null ) { String expr = completeExpressionIfAltSyntax(name); addParameter( "nameValue" , findString(expr)); } if (forAttr != null ) { addParameter( " for " , findString(forAttr)); } else { addParameter( " for " , getParameters().get( "nameValue" )); } } and now the result is like this: <label id= "labelKey" for= "baz" > baz </label> The id attribute wasn't translated which is ok, label should have a different Id than the "for" element of label, eg.: <label id= "labelKey" for= "baz" > baz </label> <input id= "baz" value= "foo" /> is it ok ?
          Hide
          Lukasz Lenart added a comment -

          First attempt to solve the problem

          Show
          Lukasz Lenart added a comment - First attempt to solve the problem
          Hide
          Lukasz Lenart added a comment -

          I added additional test case to prove that there is no problem with <s:label/> tag

          Show
          Lukasz Lenart added a comment - I added additional test case to prove that there is no problem with <s:label/> tag
          Hide
          Hudson added a comment -

          Integrated in Struts2 #530 (See https://builds.apache.org/job/Struts2/530/)
          WW-3144 Adds additional test case to prove that there no problem (Revision 1384088)

          Result = SUCCESS
          lukaszlenart :
          Files :

          • /struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java
          • /struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/ui/LabelTest.java
          • /struts/struts2/trunk/core/src/test/resources/org/apache/struts2/views/jsp/ui/Label-6.txt
          Show
          Hudson added a comment - Integrated in Struts2 #530 (See https://builds.apache.org/job/Struts2/530/ ) WW-3144 Adds additional test case to prove that there no problem (Revision 1384088) Result = SUCCESS lukaszlenart : Files : /struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/AbstractUITagTest.java /struts/struts2/trunk/core/src/test/java/org/apache/struts2/views/jsp/ui/LabelTest.java /struts/struts2/trunk/core/src/test/resources/org/apache/struts2/views/jsp/ui/Label-6.txt

            People

            • Assignee:
              Lukasz Lenart
              Reporter:
              Dave Newton
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development