Attributes behavior for empty string or space-only values.

donquixote commented 6 years ago

Currently, the Attributes class is not really designed for attribute values that are empty strings, or that contain spaces.

It might accidentally do what we expect in many cases, but it is not really guaranteed.

Different nature of attributes

The main problem is the duality of the attributes concept:

We think of attributes like 'class' as arrays of fragments/parts, to be imploded with single space. We usually want such attributes to be trimmed. We never want an empty string, a value that only contains spaces, any leading or trailing spaces, or any consecutive spaces. We don't want any duplicate fragments. Order can be alphabetical.
```
<div class="aa bb cc"></div>
```
There are attributes which contain parts joined with spaces, but where each parts may itself contain spaces which should NOT be ripped apart. We don't want any duplicate fragments, but the order of fragments might be important. On the other hand, people are not forced to specify style as fragments. In fact, they probably won't.
```
<div style="display: none; background: red; content: "bla bla"></div>
```
Perhaps there are attributes which are made up of fragments, but where duplicate fragments are allowed and must not be removed? I cannot think of any.. And again, people are not forced to use this mechanism.
We think of other attributes as standalone strings, which may contain spaces in whichever place we want.
```
<img title="Do not download and share this image, or else!" src=".."/>
```
And of course we have the boolean attributes with no value at all.
```
<checkbox disabled readonly/>
```

I think we currently don't and we should not make a hardcoded distinction based on the attribute name.

Instead, if we need to make a distinction (which is to be discussed), we should make it based on the method being called, the arguments passed to it, and the value already in the attribute.

Storage format

The storage proposed in #130 works for all cases:

$this->storage['class'] = ['aa' => 'aa', 'bb' => 'bb'] for multi-fragment attributes.
$this->storage['title'] = ['Do not download' => 'Do not download'] for one-piece attributes.
$this->storage['disabled'] = TRUE for "boolean" attributes.

However this storage does not tell us whether an attribute is intentionally single-value or just an array-like attribute which incidentally has only one fragment.

The storage for empty string is not really clear yet, it would be one of these:

$this->storage['title'] = ['' => '']
$this->storage['title'] = []

Modifier methods

Should any methods explode strings passed to them?

setAttributes(), offsetSet(), setAttribute(), merge(): If attribute values are arrays, they are understood as fragments to be joined with ' ' later. If attribute values are strings, they should be seen as single values. But should any strings nested in arrays be exploded, ever?
append(): Parameters should be seen as parts/fragments. But should anything be exploded or trimmed?
replace(): Parameters should be seen as parts/fragments.
remove(): Parameters should be seen as parts/fragments.

Proposed solution

Whenever a method is called that sets a single string value, it will do just that. No trimming, no exploding.

A method that adds fragments, e.g. setAttribute('class', ['a', 'b', 'c']) or append('class', 'x') should not explode its values either.

Or should it?

@drupol

drupol commented 6 years ago

I do agree, we should not explode the values.

donquixote commented 6 years ago

Also not trim, right?

ec-europa / atomium