javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-1966

Composite component default value resolved wrong

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.1.0
    • Fix Version/s: None
    • Component/s: facelets
    • Labels:
      None
    • Status Whiteboard:
      Hide

      size_medium importance_medium

      Show
      size_medium importance_medium

      Description

      See the attachment please.

      Or https://github.com/aliok/JSF2-Experiments/tree/master/bug2

      When I set the default value of a composite attribute to a list, it is resolved as Object.

      See the screenshot please. I'd expect the third and fourth tables to look like the first one.

      1. proposal_v1_changebundle.txt
        6 kB
        ioss
      2. rc_v2_changebundle.txt
        14 kB
        ioss
      1. bug2.png
        28 kB

        Issue Links

          Activity

          Hide
          aliok_com_tr added a comment -

          Hmm, name of the issue should be "Composite component attribute default value resolved wrong". Sorry.

          Show
          aliok_com_tr added a comment - Hmm, name of the issue should be "Composite component attribute default value resolved wrong". Sorry.
          Hide
          ioss added a comment -

          Patch proposal for a fix for #1966 (Tests not yet complete).

          The problem was not that the default List was provided as an Object, but that the ValueExpressionHandler for the 'default' attributes value was of evaltype 'String.class'.
          Switching it to evaltype 'Object.class' fixes the provided test case.

          BUT:

          As discussed on IRC, for a complete fix, the value of the type-attribute of the attribute-tag should be used as the evaltype (or 'Object.class' if no type is given, according to the specs)

          Show
          ioss added a comment - Patch proposal for a fix for #1966 (Tests not yet complete). The problem was not that the default List was provided as an Object, but that the ValueExpressionHandler for the 'default' attributes value was of evaltype 'String.class'. Switching it to evaltype 'Object.class' fixes the provided test case. BUT: As discussed on IRC, for a complete fix, the value of the type-attribute of the attribute-tag should be used as the evaltype (or 'Object.class' if no type is given, according to the specs)
          Hide
          ioss added a comment - - edited

          Fix for this issue and partially for JAVASERVERFACES-1986 (see comment there).

          Attached: rc_v2_changebundle.txt and rc_v2_newfiles.zip.

          This fix will also take into account for the type or coercion of the default-attributes value, the provided type-attribute. If no type-attribtue is provided, then the type will default to Object.class instead of String.class, as it has been the case before this fix.

          • Including Tests and code-comments.
          • Smoke-Test passed.

          Requesting review and also review for #1986,
          Imre

          Show
          ioss added a comment - - edited Fix for this issue and partially for JAVASERVERFACES-1986 (see comment there). Attached: rc_v2_changebundle.txt and rc_v2_newfiles.zip. This fix will also take into account for the type or coercion of the default-attributes value, the provided type-attribute. If no type-attribtue is provided, then the type will default to Object.class instead of String.class, as it has been the case before this fix. Including Tests and code-comments. Smoke-Test passed. Requesting review and also review for #1986, Imre
          Hide
          Ed Burns added a comment -

          Nicely done. r=edburns.

          Please make sure that smoketest.with.container.refresh.and.generate.reports has no failures before commit.

          Thank you very much.

          Show
          Ed Burns added a comment - Nicely done. r=edburns. Please make sure that smoketest.with.container.refresh.and.generate.reports has no failures before commit. Thank you very much.
          Hide
          ioss added a comment -

          Fix applied, so closing as fixed.

          Show
          ioss added a comment - Fix applied, so closing as fixed.
          Hide
          aliok_com_tr added a comment -

          What is the fix version for the issue? Do you remember?

          Show
          aliok_com_tr added a comment - What is the fix version for the issue? Do you remember?
          Hide
          christophe.rousson added a comment -

          As of version 2.1.13, class PropertyHandlerManager still contains the line :

              ALL_HANDLERS.put("default", new StringValueExpressionPropertyHandler());
          

          The attached patch seems not to have been applied in the main branch.

          May be this issue should be reopened and the patch re-applied ?

          Show
          christophe.rousson added a comment - As of version 2.1.13, class PropertyHandlerManager still contains the line : ALL_HANDLERS.put( " default " , new StringValueExpressionPropertyHandler()); The attached patch seems not to have been applied in the main branch. May be this issue should be reopened and the patch re-applied ?
          Hide
          fabmars added a comment -

          I concur.

          Show
          fabmars added a comment - I concur.
          Hide
          fabmars added a comment -

          By the way if this fix makes it again in the code, then http://docs.oracle.com/javaee/6/javaserverfaces/2.1/docs/vdldocs/facelets/composite/attribute.html will have to be changed (javax.el.ValueExpression (must evaluate to java.lang.String) is no longer true)

          Show
          fabmars added a comment - By the way if this fix makes it again in the code, then http://docs.oracle.com/javaee/6/javaserverfaces/2.1/docs/vdldocs/facelets/composite/attribute.html will have to be changed (javax.el.ValueExpression (must evaluate to java.lang.String) is no longer true)

            People

            • Assignee:
              ioss
              Reporter:
              aliok_com_tr
            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: