search

libreform / wp-libre-form

Easy native HTML5 forms for WordPress. Version 1.5 is unmaintained, but works without issue. 2.0 has been rewritten from the ground, and can be found at https://github.com/libreform/libreform
https://wordpress.org/plugins/wp-libre-form
GNU General Public License v3.0
67 stars 27 forks source link

Ensure that dynamic values and polylang feature regex doesn't break forms with inline styles #147

Closed k1sul1 closed 5 years ago

k1sul1 commented 5 years ago

I got a report from someone saying that using % in a form stops the form from being rendered.

Example: <label style="font-weight:bold; font-size:120%"><input type="text" name="doge"></label>

It was an assumption of mine that everyone would use classes and ids to style their forms, but I was wrong on that.

The current workaround is to either disable the feature by overwriting the class, or not using inline styles with % signs.

The proper fix is probably changing the regular expressions, and I suck at regular expressions. Pls help.

https://github.com/libreform/wp-libre-form/blob/master/classes/class-wplf-dynamic-values.php#L6 https://github.com/libreform/wp-libre-form/blob/master/classes/class-wplf-polylang.php#L6

The user who reported this was apparently having this issue with the dynamic values feature, but it's possible that it occurs when the Polylang feature is used, so it's worth taking a look at both of them.

k1sul1 commented 5 years ago

image

k1sul1 commented 5 years ago

Polylang isn't affected, as it uses {{ }} as matchers.

k1sul1 commented 5 years ago

/"%[^%%\n]+%/ looks like it would "handle" the situation, but that would break <input %SOMETHING%>, as it depends on double quote. [" ](%[^%%\n]+%) looks even more terrifying, but it seems to work in the cases I can imagine happening:

image

But there's always going to be a corner case that breaks it. I think we have to make a breaking change to get this to work properly. Changing % to ## or something should do the trick, 1.5 was kinda released already, so uh...

Can we call the next release 2.0? It's a lot of features we've shipped and I'd like to follow semver to at least some degree.

timiwahalahti commented 5 years ago

Changing the matcher sounds reasonable to me, even it will cause a breaking change. By continuing using % we will set ourselves to harm's way because it might cause unpredictable issues later on. We also need to ship another breaking change, #144.

Versioning next release to 2.0 👌