Forum OpenACS Development: Re: Widgets h5date and h5time : html attribute values are duplicated

Dear Khy,

thanks for your detailed report. This is a consequence of the reform discussed at https://openacs.org/forums/message-view?message_id=5799349.

Any chance you can have a look at the "larger problem" discussed there and come up with a better fix?

One "simple" fix, but not necessarily an ideal one, is that the merge_tag_attributes operation was somewhat "idempotent" e.g. we could call it multiple times without the duplication effect.

Ciao

Antonio

In https://cvs.openacs.org/changelog/OpenACS?cs=MAIN%3Aantoniop%3A20250206141115 I made so that now element(html) is consumed when attributes are merged.

This seems to be working fine for the case you have reported and on my test instance. Please let me know in case it is not.

All the best

Antonio

Hi Antonio,

I see your fix and it does work. A couple of questions :

  1. template::widget::merge_tag_attributes, is removing the element(html) is an expected behavior. Add to procedure documentation
  2. template::widget::input type element_reference tag_attributes - is procedure expecting the element(html) to be empty?

One of the solutions we tested is to pass the tag_attributes dictionary along with the element parameters, instead of using the attributes dictionary. This works on the assumption that template::widget::input will handle the merging html tags when building the widget. This works for the h5time, widget. Also, the code for template::widget::radio_text and template::widget::checkbox_text calls template::widget::input (both widgets do not have the duplicate value) with the tag_attributes instead of the attributes.

See the proposed changes "-->"

ad_proc -public template::widget::h5date {
...
} {

 upvar $element_reference element

    set attributes \
        [::template::widget::merge_tag_attributes element $tag_attributes]

    # Add fallback pattern attribute. Note that this pattern won't
    # account for leap years or invalid days of the month. We leave
    # this fine-graned validation to the server-side for now.

-->    dict set tag_attributes pattern {[0-9]+-(1[0-2]|0[0-9])-(3[0-1]|[0-2][0-9])}

    # check min/max constraint
    set last_date ""
    foreach d {max value min} {

        if {[info exists element($d)] && $element($d) ne ""} {
            set attr_value $element($d)
        } elseif {[dict exists attributes $d] && [dict get $attributes $d] ne ""} {
            set attr_value [dict get $attributes $d]
        } else {
            continue
        }

        set invalid_date_p [catch {
            set current_date [clock scan $attr_value -format "%Y-%m-%d"]
        }]

        if {!$invalid_date_p} {
            if {$last_date ne "" &&
                $current_date > $last_date} {
                ns_log Warning "template::widget::h5date value of attribute \"$d\" $attr_value too big"
            } else {
-->                dict set tag_attributes $d $attr_value
            }

            set last_date $current_date

        } else {
            ns_log Warning "template::widget::h5date value of attribute \"$d\" $attr_value is not a correct date"
        }
    }

    if {[info exists element(step)]} {
        if {[string is integer -strict $element(step)]} {
-->            dict set tag_attributes step $element(step)
        } else {
            ns_log Warning {template::widget::h5date value of attribute "step" is not an integer!}
        }
    }

 -->   return [template::widget::input date element $tag_attributes]
Dear Khy,

thanks for your feedback, I have better documented the behavior of the proc in https://cvs.openacs.org/changelog/OpenACS?cs=MAIN%3Aantoniop%3A20250207102955

If the change works for you, given that it is simple enough, I would prefer it to going after the various tag_attributes fiddlings we have in the code.

I agree that consuming element(html) is controversial from a logical standpoint, as this is a property of the widget itself and not some volatile value. A different approach could be to flip the merging logics, so that we do not return a new dict, but set the result of the merge into element(html) and use that instead...

If you come up with a better approach, feel free to provide a patch 😊

Ciao

Antonio

Thank you for adding the documentation and fix.

On the other two widgets checkbox_text ( template::widget::select_text) and radio_text ( template::widget::radio_text)
Does the call to widget::input pass in 'attributes' instead of tag_attributes to build the 'other' input?

==========
append output "$element(other_label): "
set element(value) $text
set element(name) $element(name)\.text
- append output [template::widget::input text element $tag_attributes]
+ append output [template::widget::input text element $attributes]
============