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

Hi,

template::widget::h5time and template::widget::h5date seem to be duplicating the html attribute values.

tcl code

ad_form -name demo {

} -form {
     ...
    {activity_end_date:text(h5date),optional
        {label "Ending date:"}
        {html {id activity_end_date hello world}}
    }
   ..
}  

output

<input type="date" name="activity_end_date" value="" hello="world world" id="activity_end_date activity_end_date" pattern="[0-9]+-(1[0-2]|0[0-9])-(3[0-1]|[0-2][0-9])">

The values for the attributes hello and id are duplicated.

Debugging -> There is one change in the template widget (procedure template::widget::h5date) from a previous release (5.10.1b4). It uses dict set in the procedure and then a call to template::widget::input at the end. => dict set attributes {*}$element(html)

versus in 5.10.1 , there is a call to the procedure ::template::widget::merge_tag_attributes at the top which appends dict lappend tag_attributes and then at the end calls template::widget::input (which also calls ::template::widget::merge_tag_attributes ), which results in duplicate values.

These are the procedures that call "template::widget::input" and from reviewing the code would have the same issue with duplicate values for input attributes defined with html option

  • template::widget::select_text
  • template::widget::radio_text
  • template::widget::checkbox_text
  • template::widget::h5time
  • template::widget::h5date

Thank you.

Regards, Khy

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]
============