Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor selection of % as placeholder delimiter because is commonly used in CSS and JavaScript code. #1249

Open
aiotech-pub opened this issue Dec 4, 2022 · 15 comments
Labels

Comments

@aiotech-pub
Copy link

% is used commonly in CSS to define style and aspect ratio of elements related to the parent. It is also the modulus operator in JavaScript. This means that any couple of % inside a String or HTML file where is present at least one place holder can not have any other % inside because they will be wrongly interpreted as the delimiters of a placeholder. In my opinion a single character can not be used as delimiter so I strongly suggest a couple of characters as may be %%.

@aiotech-pub
Copy link
Author

At the moment to quickly solve my problem I replaced % with _ and everything works now.

@Sveninndh
Copy link

Sveninndh commented Dec 8, 2022

I ran into the same problem, it took me a few hours. When studying the source code, I noticed two things that are unfortunately not documented.
1.) The maximum length of a placeholder is limited to 32 bytes.

2.) If you use % within a string or HTML file,
you must escape all occurrences of % - which are not enclose a placeholder - to %%.
Example: 'Width: 50%;' needs to be changed to 'width: 50%%;'

@hmueller01
Copy link

@Sveninndh Thanks for your posting. This helped me a lot! I would suggest opening a PR to add this to the README.md but I doubt it will be merged as @me-no-dev didn't do anything since a year ...

@DeeEmm
Copy link

DeeEmm commented Dec 19, 2022

This has been mentioned several times before. I also reported this issue just a short time ago...

#1234

Escaping the % characters is little more than a hack as it generates non-compliant css... any solution that creates another problem is not a solution at all ;)

@Sveninndh
Copy link

@DeeEmm Why do you think it would generates non-compliant css?

After processing the callback template processor the result should be compliant css.
Or do I miss something?

@DeeEmm
Copy link

DeeEmm commented Dec 19, 2022

Incorrect source syntax. Try it in a validator.

% is not a valid escape character for CSS

W3C recommends that it is "always preferable to display characters in their original form" because "using escapes can make it difficult to read and maintain source code, and can also significantly increase file size."

Compliant code is more than what you send to the browser. I mean, lets just obscure it by using CSS escapes and Gzipping it, after all if it is non-human readable no one will know, right?

Semantics are very important. Remember IE6??

Coding standards exist for a reason.

/DM

@Sveninndh
Copy link

Some valid points.
try my fix in the attachment and tell me if it works.
The idea is that the parameter names have a maximum of 32 characters and contain no illegal characters.
WebResponses.zip

@DeeEmm
Copy link

DeeEmm commented Dec 20, 2022

Thanks @Sveninndh much appreciated.

I've taken a quick flick through your updated code. Would I be correct in assuming you are implementing opening and closing tags. IMHO this is the correct way to implement templating, especially if using multi character tags.

Unfortunately for me I use this library in a public repo so am tied to using unmodified vanilla source, otherwise I'd have to maintain modified libraries on top of my main repo. If it was a personal project a solution like yours would be perfect.

What I ended up doing in my case was to use the vh / vw units (viewport height / width) for dynamic element scaling instead of percent as this is at least valid css. Not the best solution I know, as these units are unfamiliar to many but this along with a note in the header should be enough to prevent potential future issues.

As an aside I've also moved to use the esphome fork of both this and the AsyncTCP libraries as they have integrated many of the unmerged fixes from this repo. Unfortunately these do not address this particular issue, but they do address many others. The esphome repos are also also actively maintained as the esphome home automation community is both large and mature.

Perhaps create a pull request for your code there? The only other consideration I would mentions is backwards compatibility, i.e. does it still work with % tags? as this would be a requirement to avoid creating a breaking change.

esphome/AsyncTCP-esphome
esphome/ESPAsyncWebServer-esphome

/DM

@Sveninndh
Copy link

does it still work with % tags?

Yes, it should still work.
Maybe I should create a push-request at [me-no-dev] first.

@DeeEmm
Copy link

DeeEmm commented Dec 21, 2022

Maybe I should create a push-request at [me-no-dev] first.

For sure, however I'm not sure it would ever get merged. This repo appears to be no longer actively maintained. Apart from the many unanswered support requests, there are heaps of pull requests addressing issues that have not been merged, most of which are now out of date with the main branch. The last merge was nine months ago.

It's a similar story over on Gitter. Last active back in October.

I think he's tied up with other projects.

/DM

Sveninndh added a commit to Sveninndh/ESPAsyncWebServer that referenced this issue Jan 3, 2023
…dev#1249)

AsyncAbstractResponse::_fillBufferAndProcessTemplates()

1.) A parameter is enclosed by the % delimiter.
2.) Parameter names can have a maximum of 32 characters.
3.) Parameter names cannot contain illegal characters. (\ .:<>{}',;"/)

A parameter is only recognized and replaced if all of these conditions are met.
Otherwise the text source code is not changed. Using double % is no longer necessary.
Nevertheless, this mechanism will continue to be supported for reasons of compatibility.
@radozebra
Copy link

just set the TEMPLATE_PLACEHOLDER build flag to replace the character with whatever you want. I have it set to # since I always use the rgb() to set the colors in css.
image

@DeeEmm
Copy link

DeeEmm commented Jan 5, 2023

just set the TEMPLATE_PLACEHOLDER build flag to replace the character with whatever you want.

@radozebra Thanks for this solution, I'm going to give this a try.

@DeeEmm
Copy link

DeeEmm commented Jan 5, 2023

I can confirm that this is working for me. Thanks for your solution. Very much appreciated.

@Jakeduncan00
Copy link

Jakeduncan00 commented Jan 6, 2023

The ::placeholder selector selects form elements with placeholder text, and let you style the placeholder text. The placeholder text is set with the placeholder attribute, which specifies a hint that describes the expected value of an input field. Krowd Darden App

@stale
Copy link

stale bot commented May 21, 2023

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants