html-purify icon indicating copy to clipboard operation
html-purify copied to clipboard

Add proper quotes

Open adon-at-work opened this issue 9 years ago • 6 comments

@maditya please review

I addressed the problems stated in https://github.com/yahoo/html-purify/pull/21 with an approach that centralizes a single place to capture and treat attribute values.

c.c @yukinying @neraliu

adon-at-work avatar Aug 11 '15 11:08 adon-at-work

@maditya , i made this PR really out of my curiosity. would love to see how such an impl will perform, and how cleaner the coding can be, if I load a single transition with multiple actions. i like your original approach too. so, just feel free to co-edit/fork this branch, copy anything to yours, or we could co-edit your branch. :)

let's also discuss (perhaps in the standup meeting) how or whether we should balance optional tags, as described in https://github.com/yahoo/html-purify/commit/dd4ae8b7ba818b99169f0a2575adde54b5ae2794#diff-9c568acf92dc1a69fdd058c41195c719R139

adon-at-work avatar Aug 12 '15 04:08 adon-at-work

CLA is valid!

yahoocla avatar Aug 12 '15 04:08 yahoocla

@yukinying, https://github.com/yahoo/html-purify/commit/a2d40267a0c37f7f3c10fae5456eee8dbe035335 is the patch using the "incremental" bit encoding to represent different actions. In particular, the higher group actually do not actually look incremental (i.e., 0x08, 0x10, 0x18, ...). Unless we give up the forth bits to make it like 0x10, 0x20, 0x30, ..., which ultimately isn't much more space efficient than the original. that will become https://github.com/yahoo/html-purify/commit/4046a9e90a2270cb4c180502006bdfd3c2c1475d I personally think the first one is a little bit ugly, and I liked the original better. both worked. just let me know which is gd so that we can go ahead. thanks :)

adon-at-work avatar Oct 05 '15 16:10 adon-at-work

We would like to keep the design simple and straightforward, so the ideal implementation is to keep lower group and higher group in separate variable. It's fine to compress lower group and higher group together in a variable, but I don't like the idea of converting them to bitwise flag. Those variables are supposed to store state number rather than flags.

yukinying avatar Oct 06 '15 03:10 yukinying

tiny cleaned up. git rebased to master

adon-at-work avatar Oct 06 '15 04:10 adon-at-work

Further to https://github.com/yahoo/html-purify/pull/22#issuecomment-145594920, I think https://github.com/yahoo/html-purify/commit/a2d40267a0c37f7f3c10fae5456eee8dbe035335 is meant to show the ugliness. https://github.com/yahoo/html-purify/commit/4046a9e90a2270cb4c180502006bdfd3c2c1475d is what we now prefer, therefore, having both lower and higher groups to be sequential. But when we one day really run out of spaces, and have to encode a lot of actions (not states actually, notice that here the table[prevState][nextState] gives an action), we might have to fall back to encode using the ugly way.

I didn't realize this during our prior discussion but only when I implement it. So, let's see if we've any new thoughts/inputs to this problem. If not, we can stick with the 4046a9e as discussed.

adon-at-work avatar Oct 06 '15 04:10 adon-at-work