Refactor "is attribute allowed"#385
Conversation
|
I have been thinking it might be cleaner to add a new algorithm like is attribute allowed. This might also be useful for #387, because we could use it like "is attribute allowed |
Done. Please take another look. |
|
Should we move the |
Done. |
|
Sorry, I just realized that this won't really work for my idea of using this for implementing a manual |
evilpie
changed the title
|
@otherdaniel Can I help getting this done? |
Please give it another look. Apologies for being a bit late with this. |
| «[|elementName|, |attrName|]» and |attr|'s | ||
| [=get an attribute value|value=] [=string/is=] "`href`" or | ||
| "`xlink:href`", then [=/remove an attribute|remove=] |attribute|. | ||
| 1. Let |attrName| be a {{SanitizerAttributeNamespace}} with |attribute|'s |
There was a problem hiding this comment.
Let's move this before is attribute allowed, and then call it with |attrName|.
There was a problem hiding this comment.
Done. (Also calling it with |elementName| for consistency.)
There was a problem hiding this comment.
The error in the preview script seems unrelated to this change. If I've traced it correctly, bikeshed seems to expect boilerplate files in bikeshed/spec-data/boilerplate, but they're actually installed in bikeshed/spec-data/readonly/boilerplate. No idea why...
…, rather then DOM nodes.
| 1. Otherwise: | ||
| 1. If |elementWithLocalAttributes|["{{SanitizerElementNamespaceWithAttributes/attributes}}"] | ||
| [=map/exists=] and |elementWithLocalAttributes|["{{SanitizerElementNamespaceWithAttributes/attributes}}"] does not [=SanitizerConfig/contain=] |attrName|: | ||
| 1. Return [=/blocked=]. |
There was a problem hiding this comment.
You seem to have dropped the check for SanitizerConfig/removeAttributes?
There was a problem hiding this comment.
You're right! Should now be fixed.
| 1. If |elementWithLocalAttributes|["{{SanitizerElementNamespaceWithAttributes/attributes}}"] | ||
| [=map/exists=] and |elementWithLocalAttributes|["{{SanitizerElementNamespaceWithAttributes/attributes}}"] does not [=SanitizerConfig/contain=] |attrName|: | ||
| 1. Return [=/blocked=]. | ||
| 1. Otherwise, if |configuration|["{{SanitizerConfig/removeAttributes}}"] |
There was a problem hiding this comment.
| 1. Otherwise, if |configuration|["{{SanitizerConfig/removeAttributes}}"] | |
| 1. If |configuration|["{{SanitizerConfig/removeAttributes}}"] |
Early return now.
3 participants
Split the dreaded multi-line condition into several subclauses, assign each to an appropriately named boolean, and then test those bools.
That's more verbose than I usually go for, but it's surely more readable than the old version.
Fix: #380
Preview | Diff