Tapestry
  1. Tapestry
  2. TAPESTRY-975

Overriding ValidationDelegate and adding a "class" attribute results in duplicate attributes

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0.2
    • Fix Version/s: 4.1.5
    • Component/s: Framework
    • Labels:
      None

      Description

      I have the following in my custom ValidationDelegate:

      public void writeAttributes(IMarkupWriter writer, IRequestCycle cycle,
      IFormComponent component, IValidator validator) {
      if (isInError())

      { String cssClass = ((component.getBinding("class") != null) ? component.getBinding("class").getObject().toString() : ""); writer.attribute("class", cssClass + " error"); }

      }

      However, rather than just writing a single "class" attribute, it writes two:

      class="text large error" class="text large"

      Ideally, only one "class" attribute gets written. Maybe IMarkupWriter nees an appendAttribute() method, or it just needs to be smart enough to detect duplicates?

        Issue Links

          Activity

          Matt Raible created issue -
          Hide
          Matt Raible added a comment -

          This should probably be marked as a Minor issue, but it doesn't appear like I have rights to do that.

          Show
          Matt Raible added a comment - This should probably be marked as a Minor issue, but it doesn't appear like I have rights to do that.
          Andreas Andreou made changes -
          Field Original Value New Value
          Link This issue duplicates TAPESTRY-550 [ TAPESTRY-550 ]
          Andreas Andreou made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Duplicate [ 3 ]
          Fix Version/s unspecified [ 10790 ]
          Hide
          Andreas Andreou added a comment -

          This is not really a dup of TAPESTRY-550, so reopenning...

          Show
          Andreas Andreou added a comment - This is not really a dup of TAPESTRY-550 , so reopenning...
          Andreas Andreou made changes -
          Resolution Duplicate [ 3 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Andreas Andreou made changes -
          Issue Type Bug [ 1 ] New Feature [ 2 ]
          Andreas Andreou made changes -
          Link This issue duplicates TAPESTRY-550 [ TAPESTRY-550 ]
          Andreas Andreou made changes -
          Link This issue relates to TAPESTRY-550 [ TAPESTRY-550 ]
          Jesse Kuhnert made changes -
          Assignee Jesse Kuhnert [ jkuhnert ]
          Jesse Kuhnert made changes -
          Fix Version/s 4.1.1 [ 12312021 ]
          Fix Version/s unspecified [ 10790 ]
          Hide
          Greg Woolsey added a comment -

          The example looks incorrect to me, assuming implicit attributes are written out for this component, then the code should only update the binding, not explicitly write out an attribute that will be written out elsewhere as part of the implicit parameters.

          I think this is a coding problem, not a bug with Tapestry.

          Show
          Greg Woolsey added a comment - The example looks incorrect to me, assuming implicit attributes are written out for this component, then the code should only update the binding, not explicitly write out an attribute that will be written out elsewhere as part of the implicit parameters. I think this is a coding problem, not a bug with Tapestry.
          Hide
          Andreas Andreou added a comment -

          Nope, this is not a coding problem.
          Updating the binding isn't a solution cause:

          • users supplying the class parameter do NOT expect the value they pass to change
          • class parameter is actually an informal parameter - no way to alter its binding - that i know of

          A small cache that would hold attributes of the currently opened tag and would write them out
          all at once when the begin-tag closes (instead of writing the attributes on-demand) would help...
          but of course this might decrease performance. That's probably why noone has tackled this.

          Show
          Andreas Andreou added a comment - Nope, this is not a coding problem. Updating the binding isn't a solution cause: users supplying the class parameter do NOT expect the value they pass to change class parameter is actually an informal parameter - no way to alter its binding - that i know of A small cache that would hold attributes of the currently opened tag and would write them out all at once when the begin-tag closes (instead of writing the attributes on-demand) would help... but of course this might decrease performance. That's probably why noone has tackled this.
          Hide
          Jesse Kuhnert added a comment -

          I agree with the original issue reported.

          calling attribute() now overrides any previous attributes with the same name.

          There are also all manner of new methods available like appendAttribute/get/has/clear/remove /etc..

          Show
          Jesse Kuhnert added a comment - I agree with the original issue reported. calling attribute() now overrides any previous attributes with the same name. There are also all manner of new methods available like appendAttribute/get/has/clear/remove /etc..
          Jesse Kuhnert made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Resolved [ 5 ]
          Hide
          Matt Raible added a comment -

          In 4.1.3, when I have a label:

          <label class="desc" jwcid="@FieldLabel" field="component:usernameField">Username</label>

          The following doesn't work to produce <label class="desc error">

          @Override
          public void writeLabelAttributes(IMarkupWriter writer, IRequestCycle cycle, IFormComponent component) {
          if (isInError())

          { writer.appendAttribute("class", "error"); }

          }

          Nor does writer.attribute:

          @Override
          public void writeLabelAttributes(IMarkupWriter writer, IRequestCycle cycle, IFormComponent component) {
          if (isInError())

          { writer.attribute("class", "error"); }

          }

          Show
          Matt Raible added a comment - In 4.1.3, when I have a label: <label class="desc" jwcid="@FieldLabel" field="component:usernameField">Username</label> The following doesn't work to produce <label class="desc error"> @Override public void writeLabelAttributes(IMarkupWriter writer, IRequestCycle cycle, IFormComponent component) { if (isInError()) { writer.appendAttribute("class", "error"); } } Nor does writer.attribute: @Override public void writeLabelAttributes(IMarkupWriter writer, IRequestCycle cycle, IFormComponent component) { if (isInError()) { writer.attribute("class", "error"); } }
          Matt Raible made changes -
          Status Resolved [ 5 ] Reopened [ 4 ]
          Resolution Fixed [ 1 ]
          Hide
          Matt Raible added a comment -

          It seems that having a "class" attribute causes writer.attribute and writer.appendAttribute to not work. Here's my Tapestry code:

          <li>
          <label class="desc" jwcid="@FieldLabel" field="component:usernameField">Username</label>
          <input jwcid="usernameField" type="text" id="username" class="text large"/>
          </li>

          This produces the following when there's an error (notice no new classes in the "class" attribute):

          <li>
          <label for="username" class="desc">Username<span class="req"> *</span></label>
          <span class="fieldError"><img src="/images/iconWarning.gif" class="validationWarning" alt="Warning"></img> You must enter a value for Username.</span><input type="text" name="username" value="" id="username" class="text large" />
          </li>

          The extra HTML is because I've overridden the "writePrefix" method.

          If I remove the "class" attributes of <label> and <input>, it seems to work properly:

          <li>
          <label jwcid="@FieldLabel" field="component:usernameField">Username</label>
          <input jwcid="usernameField" type="text" id="username"/>
          </li>

          Produces the following when there's an error:

          <li>
          <label for="username" class="error">Username<span class="req"> *</span></label>
          <span class="fieldError"><img src="/images/iconWarning.gif" class="validationWarning" alt="Warning"></img> You must enter a value for Username.</span><input type="text" name="username" value="" id="username" class="fieldMissing" />
          </li>

          I've attached my ValidationDelegate.java class.

          Show
          Matt Raible added a comment - It seems that having a "class" attribute causes writer.attribute and writer.appendAttribute to not work. Here's my Tapestry code: <li> <label class="desc" jwcid="@FieldLabel" field="component:usernameField">Username</label> <input jwcid="usernameField" type="text" id="username" class="text large"/> </li> This produces the following when there's an error (notice no new classes in the "class" attribute): <li> <label for="username" class="desc">Username<span class="req"> *</span></label> <span class="fieldError"><img src="/images/iconWarning.gif" class="validationWarning" alt="Warning"></img> You must enter a value for Username.</span><input type="text" name="username" value="" id="username" class="text large" /> </li> The extra HTML is because I've overridden the "writePrefix" method. If I remove the "class" attributes of <label> and <input>, it seems to work properly: <li> <label jwcid="@FieldLabel" field="component:usernameField">Username</label> <input jwcid="usernameField" type="text" id="username"/> </li> Produces the following when there's an error: <li> <label for="username" class="error">Username<span class="req"> *</span></label> <span class="fieldError"><img src="/images/iconWarning.gif" class="validationWarning" alt="Warning"></img> You must enter a value for Username.</span><input type="text" name="username" value="" id="username" class="fieldMissing" /> </li> I've attached my ValidationDelegate.java class.
          Matt Raible made changes -
          Attachment ValidationDelegate.java [ 12365767 ]
          Hide
          Jesse Kuhnert added a comment -

          FieldLabel was calling renderInformalParameters after letting the validation delegate run. By default AbstractComponent calls IMarkupWriter.attribute(name, value) when writing informal parameters - thus blowing away any conflicting attributes written by IValidationDelegate. Moved the call to happen before IValidationDelegate runs.

          Show
          Jesse Kuhnert added a comment - FieldLabel was calling renderInformalParameters after letting the validation delegate run. By default AbstractComponent calls IMarkupWriter.attribute(name, value) when writing informal parameters - thus blowing away any conflicting attributes written by IValidationDelegate. Moved the call to happen before IValidationDelegate runs.
          Jesse Kuhnert made changes -
          Resolution Fixed [ 1 ]
          Status Reopened [ 4 ] Resolved [ 5 ]
          Fix Version/s 4.1.4 [ 12312763 ]
          Fix Version/s 4.1.1 [ 12312021 ]
          Mark Thomas made changes -
          Workflow jira [ 12372721 ] Default workflow, editable Closed status [ 12568219 ]
          Mark Thomas made changes -
          Workflow Default workflow, editable Closed status [ 12568219 ] jira [ 12590047 ]

            People

            • Assignee:
              Jesse Kuhnert
              Reporter:
              Matt Raible
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development