ClassApplier should check Attributes during applyToTextNode() and isRemovable()

[betson]

Source: https://github.com/timdown/rangy/blob/780f783/src/js/modules/rangy-cssclassapplier.js#L681-L684

Within createContainer(), we ensure the new element has all of the expected properties and attributes. If useExistingElements, then we should maintain consistency by ensuring the existing elements already have the requested attributes.

This should be done in applyToTextNode() and isRemovable().

[timdown]

I seem to have made a pretty half-hearted job of implementing elementAttributes and I'm not entirely sure what my intentions were when I did it since I haven't documented it anywhere. I suspect I didn't want to get into the attribute/property area: the relationship between attributes and properties is slightly complicated and vastly more complicated in old IE and I really don't want to be writing large chunks of code to deal with it. My ideal would be to get rid of elementAttributes.

[tobiasschweizer]

I agree that this issue is related to the one I created (#213).

In my pull request #217, I added some missing code for the elementAttributes handling

Please note that I need the elementAttributes. Sometimes it is useful to add specific information to an element which makes it different from other elements having although sharing the same class

[timdown]

There is already elementProperties which can be used to set most attribute values.

I will think about this properly. I'd like to postpone doing this until the next version: 1.3 has been hanging around in alpha for a ridiculous amount of time now.

[betson]

I second the value in elementAttributes. I personally use it to apply data- HTML attributes onto an element. When using multiple ClassAppliers for the same CSS class, this is useful for providing unique information onto a created element.

My temporary workaround involves always setting useExistingElements to false when attributes are needed.

[tobiasschweizer]

Is there an example how to use elementProperties? I want to set an HTML5-compliant attribute (data-xy).

[sashless]

To apply data-x you need to use elementAttributes in options object @tobiasschweizer

[timdown]

Here is a summary of the issues, which is as much for my benefit as anything else:

elementProperties has existed since version 1.2 and works as follows for a class applier with class "someClass":

• All properties are added to any new element created with class "someClass"
• For an existing element to be reused when applying the class, the element's properties must match elementProperties. Otherwise, a new element is created.
• For an element with class "someClass" to be removed when unapplying a class, the element's properties must match elementProperties. Otherwise, "someClass" is removed from the element (so long as it has the correct tagName).

I have changed the code to do the same thing for elementAttributes. However, there are potential problems with this:

• Attributes set properties. Properties sometimes set attributes. Therefore, if elementProperties has a property that matches an attribute in elementAttributes, things will almost certainly go wrong. Trying to detect or work around this is tricky because attribute values are always strings and properties can be other things (numbers, Booleans, objects).
• Old IE. setAttribute() and getAttribute() do not behave properly: they get/set a property rather than an attribute. This means that, for example, trying to create <a> elements with an href attribute will go wrong in old IE with a relative URL. An edge case, maybe, but it's the kind of thing I like to work around. However, the scope of working around every such case seems too great to me.

I will leave it as it is now and in the documentation suggest being very careful with using both elementProperties and elementAttributes.