Wicket
  1. Wicket
  2. WICKET-3884

Null model for AttributeAppender should not render empty attribute

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.5-RC5.1
    • Fix Version/s: 1.5-RC6
    • Component/s: wicket
    • Labels:

      Description

      I can't think of a reason this would be valid, but passing in null model renders <span class="">Test</span>. If previous and new attribute are both null, the component should render cleanly like <span>Test</span>.

      1. test-WICKET-3884.patch
        2 kB
        Robert McGuinness
      2. attr-modifier-by-identity.patch
        1 kB
        Martin Grigorov

        Activity

        Hide
        Pedro Santos added a comment -

        +1 to deprecate VA_ADD and VA_REMOVE in benefit of #remove(String), it is better to read.

        Show
        Pedro Santos added a comment - +1 to deprecate VA_ADD and VA_REMOVE in benefit of #remove(String), it is better to read.
        Hide
        Martin Grigorov added a comment -

        I think special values VA_ADD and VA_REMOVE should be deprecated.
        If AttributeModifier should be able to remove an attribute then explicit #remove(String) method should be added.

        Show
        Martin Grigorov added a comment - I think special values VA_ADD and VA_REMOVE should be deprecated. If AttributeModifier should be able to remove an attribute then explicit #remove(String) method should be added.
        Hide
        Martin Grigorov added a comment -

        While working on this I'd like to ask about the checks for VALUELESS_ constants: isn't it better to use checks for equality by identity instead of .equals() ?

        With .equals() it is not possible to use values 'VA_ADD' and 'VA_REMOVE'. Using new String() and == will solve this. The constants are static so this shouldn't be a problem for deserialization. What do you think ?

        Show
        Martin Grigorov added a comment - While working on this I'd like to ask about the checks for VALUELESS_ constants: isn't it better to use checks for equality by identity instead of .equals() ? With .equals() it is not possible to use values 'VA_ADD' and 'VA_REMOVE'. Using new String() and == will solve this. The constants are static so this shouldn't be a problem for deserialization. What do you think ?
        Hide
        Robert McGuinness added a comment -

        Failing test case added

        Show
        Robert McGuinness added a comment - Failing test case added

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Robert McGuinness
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development