matomo-org / tag-manager

Free Open Source Matomo Tag Manager - A simple way to manage and maintain all of your (third-party) tags on your website.
https://matomo.org
GNU General Public License v3.0
176 stars 58 forks source link

Variable type "DOM Element" cannot extract values of "input" tags #889

Open MichaelRoosz opened 2 months ago

MichaelRoosz commented 2 months ago

When trying to extract the (current) value of an "input" tag, the "DOM Element" variable type will fail, even when configured to use the attribute name "value".

This is because we use "getAttribute()" to obtain the value, which does not return the current value, just the initial one. https://github.com/matomo-org/tag-manager/blob/961de621afb9e058250f9aefd05f40a1402348dd/javascripts/tagmanager.js#L789

I am willing to contribute a fix for this, but I would like to know if and how you would like to fix this.

MichaelRoosz commented 2 months ago

As a workaround I created a Tag Manager extension plugin: https://github.com/MichaelRoosz/plugin-TagManagerInputExtension

It provides a variable to read input values and a trigger for input changes. However, in case you would like to adjust the "DOM Element" variable, I would still be willing to work on a fix for it.

snake14 commented 2 months ago

Hi @MichaelRoosz . Thank you for taking the time to create this issue and share what you've worked on. As the DOM method is getElementAttribute I think that returning the attribute value makes sense. Maybe a new getElementProperty method should be added to the DOM class. What do you think @AltamashShaikh ?

@MichaelRoosz just skimming over your plugin, I think that it looks pretty good. You're always welcome to submit those triggers and variables in a PR for our tag manager repository. We're happy to look at any PRs that you'd like to submit.

AltamashShaikh commented 2 months ago

@snake14 A PR from @MichaelRoosz would be better, as it solves the problem

AltamashShaikh commented 1 month ago

@MichaelRoosz Will you be willing to send a PR for this issue ?

MichaelRoosz commented 1 month ago

@AltamashShaikh yes, I will take care of this next week