speced / respec

A tool for creating technical documents and web standards
https://respec.org/
Other
718 stars 388 forks source link

Remove raw HTML string interpolation #2551

Open saschanaz opened 4 years ago

saschanaz commented 4 years ago

One consensus in #2548 is that we should remove raw string interpolations. Now we can easily remove them since the PR found all the raw things 🎉

saschanaz commented 4 years ago

@marcoscaceres Candidate for good first issue? 👀

marcoscaceres commented 4 years ago

I think so! We just need to make them super easy to find, and really clear how to fix them.

saschanaz commented 4 years ago

Do we want to keep conf.*HTML e.g. conf.wgHTML or not?

marcoscaceres commented 4 years ago

I don't think so... those are all part of the "override" tag'ed tasks.

saschanaz commented 4 years ago

I don't think so... those are all part of the "override" tag'ed tasks.

You mean you want to remove the overriding things? GitHub search shows only one use of wgHTML which is marked as old 👀

marcoscaceres commented 4 years ago

Maybe we can just entirely remove that.

What I mean is, for a lot of things that insert html from config, authors should be actually writing those things directly in the document. There are whole bunch of these: https://github.com/w3c/respec/issues?q=is%3Aissue+is%3Aopen+label%3AOverride

saschanaz commented 4 years ago

Right, just like how additional SOTD contents are processed.

deepesh-ludhyani commented 4 years ago

I would be glad to try to solve this issue.

saschanaz commented 4 years ago

Hello @deepesh-ludhyani, I'm assigning you for this issue then! Feel free to ask questions whenever needed. 👍

SuyashSalampuria commented 4 years ago

@saschanaz if no one is working on this, shall I try to solve this issue

saschanaz commented 4 years ago

@deepesh-ludhyani Are you working on this?

SuyashSalampuria commented 4 years ago

@saschanaz I am starting to work on this issue Can you confirm if I have understood the issue correctly Please confirm if have to convert Screenshot from 2019-11-26 23-40-25 to <li> <a href="${href}"> ${testFileName} </a> ${emojiList} </li>

saschanaz commented 4 years ago

@SuyashSalampuria No, that one is fine. What do we want is that we replace normal JavaScript strings into hyperHTML templated string so that we get all of its feature including escaping.

So for example:

const element = `<p>${content}</p>`;

Should be:

const element = html`<p>${content}</p>`;

...although it'll be more complex than this.

marcoscaceres commented 4 years ago

@saschanaz, as this is security critical, I'd be more comfortable if you were working on it.

SuyashSalampuria commented 4 years ago

I have tried my best to understand the problem and solve it. Although, this was my first contribution to open source. Please tell me if I have understood the problem correctly and solved it as required

ridhishjain-zepto commented 4 years ago

@saschanaz, can I take this issue? or anyone else is working on it?

saschanaz commented 4 years ago

Several are already removed by others, not sure whether there is a remaining. If you can find one, please do.

ridhishjain-zepto commented 4 years ago

I did not find any in src/w3c, src/ui and, src/geonovum any other directories to look for? otherwise, the issue is resolved I guess