Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text spacing not !important [78fd32, 24afc2, 9e45ec]: Consider actual font size and ignore empty text #1804

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions __tests__/spelling-ignore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@
- 2px
- 3px
- 4px
- 10px
- 16px
- 20px
- 24px
Expand Down
73 changes: 61 additions & 12 deletions _rules/letter-spacing-not-important-24afc2.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@ This rule applies to any [HTML element][] that is [visible][] and for which the

For each test target, at least one of the following is true:

- **not important**: the [computed][] value of its [letter-spacing][] property is not [important][]; or
- **wide enough**: the [computed][] value of its [letter-spacing][] property is at least 0.12 times the [computed][] value of its [font-size][] property; or
- **cascade**: the [cascaded][] value of its [letter-spacing][] property is not a value [declared][] in its `style` attribute.
- <dfn id="24afc2:not-important">not important</dfn>: the [computed][] value of its [letter-spacing][] property is not [important][]; or
- <dfn id="24afc2:spaced-text">spaced text</dfn>: for each text node descendant of this target, one of the following is true:
Jym77 marked this conversation as resolved.
Show resolved Hide resolved
- <dfn id="24afc2:empty">empty</dfn>: the text node is only [whitespace][]; or
- <dfn id="24afc2:wide-enough-cond">wide enough</dfn>: the parent of the text node has [wide enough letter spacing](#24afc2:wide-enough-def); or
- <dfn id="24afc2:cascade">cascade</dfn>: the parent of the text node has a [cascaded][] [letter-spacing][] property which is not the one [declared][] in the `style` attribute of the target.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been wrestling with this definition a bit. I finally understand it, but it seems weird since it feels like it is almost defining an inapplicable case and then calling it a pass (e.g., the text node doesn't use the target elements style which results in a pass). I'm not sure I have a suggestion for this, but figured I'd leave my thoughts in case it helps to figure out how to clarify this language somehow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the problem is that a single target can have all three cases (or more), something like:

<div style="letter-spacing: 2.4 px !important; font-size: 30px">
  <span>&nbsp;</span> <!-- OK because too small but only whitespace -->
  <span style="font-size: 20px">Lorem</span> <!-- OK because font-size has changed -->
  <span style="letter-spacing: 3.6px !important">ipsum</span> <!-- OK because letter-spacing has changed -->
  <span>dolor sit amet</span> <!-- bad -->
</div>

The 4th case totally only fails because of the div, so we do want the div as a target.

We could update applicability to say something like "…and has at least one text node descendant which is not only whitespace and whose parent's cascaded letter-spacing is not the one declared here". That would make cases with only the first or third case Inapplicable, which is quite good (these are cases where the !important declaration does not affect anything. But we would still need to keep the "empty" and "cascade" exceptions in the Expectation due to the possibility of mixing things up, as here. Hence, I feel that doing so would result in complicating the Applicability without simplifying the Expectation, and duplicating some condition (making maintenance harder).

Another solution could be to target the text nodes (which are not empty and whose parent's cascaded letter-spacing is !important and declared in a style attribute), but that would result in failing the text nodes rather than the (element with the) bad declaration, so it is not that good for reporting. This would result in only targeting "Lorem" (pass) and "dolor sit amet" (fail).


An element has a <dfn id="24afc2:wide-enough-def">wide enough letter spacing</dfn> if the computed value of its [letter-spacing][] property is at least 0.12 times the computed value of its [font-size][] property.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mirror what we did with the line height we might want to make this the used value instead of the computed value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. It is not really needed for letter-spacing and word-spacing since the used value and computed value should be equal. But alignment is a good idea…


## Assumptions

Expand Down Expand Up @@ -63,7 +67,7 @@ CSS specifications define each declaration as being either [important][] (if it

#### Passed Example 1

This `p` element has a **not [important][]** [computed][] `letter-spacing`.
This `p` element has a [not important](#24afc2:not-important) [computed][] `letter-spacing`.

```html
<p style="letter-spacing: 0.1em">
Expand All @@ -73,7 +77,7 @@ This `p` element has a **not [important][]** [computed][] `letter-spacing`.

#### Passed Example 2

This `p` element has a [computed][] `letter-spacing` of 0.15 time the font size, which is **wide enough**.
This `p` element has a [computed][] `letter-spacing` of 0.15 time the font size, which is [wide enough](#24afc2:wide-enough-cond).

```html
<p style="letter-spacing: 0.15em !important">
Expand All @@ -83,7 +87,7 @@ This `p` element has a [computed][] `letter-spacing` of 0.15 time the font size,

#### Passed Example 3

This `p` element has a [computed][] [letter-spacing][] of `3px`, which is **wide enough** (the threshold is `3px`).
This `p` element has a [computed][] [letter-spacing][] of `3px`, which is [wide enough](#24afc2:wide-enough-cond) (the threshold is `3px`).

```html
<style>
Expand All @@ -99,7 +103,7 @@ This `p` element has a [computed][] [letter-spacing][] of `3px`, which is **wide

#### Passed Example 4

This `p` element has two [declared][] values for its `letter-spacing` property. The latest wins the [cascade sort][]. It has a value of `0.15em`, and is **wide enough**.
This `p` element has two [declared][] values for its `letter-spacing` property. The latest wins the [cascade sort][]. It has a value of `0.15em`, and is [wide enough](#24afc2:wide-enough-cond).

```html
<p style="letter-spacing: 0.1em !important; letter-spacing: 0.15em !important">
Expand All @@ -109,7 +113,7 @@ This `p` element has two [declared][] values for its `letter-spacing` property.

#### Passed Example 5

This `p` element has two [declared][] values for its `letter-spacing` property. The one which is [important][] wins the [cascade sort][]. It has a value of `0.15em`, and is **wide enough**.
This `p` element has two [declared][] values for its `letter-spacing` property. The one which is [important][] wins the [cascade sort][]. It has a value of `0.15em`, and is [wide enough](#24afc2:wide-enough-cond).

```html
<p style="letter-spacing: 0.15em !important; letter-spacing: 0.1em">
Expand All @@ -119,7 +123,7 @@ This `p` element has two [declared][] values for its `letter-spacing` property.

#### Passed Example 6

The [cascaded][] value of the `letter-spacing` property of this `p` element is [declared][] in the style sheet, not in the `style` attribute (it wins the [cascade sort][] because it is [important][]). Thus, the `p` element matches the **cascade** condition.
The [cascaded][] value of the `letter-spacing` property of this `p` element is [declared][] in the style sheet, not in the `style` attribute (it wins the [cascade sort][] because it is [important][]). Thus, the `p` element matches the [cascade](#24afc2:cascade) condition.

```html
<style>
Expand All @@ -135,7 +139,7 @@ The [cascaded][] value of the `letter-spacing` property of this `p` element is [

#### Passed Example 7

The [computed][] value of the `letter-spacing` property of this `p` element is **not [important][]**. The [computed][] value of the `letter-spacing` property of this `span` element is the [inherited][] value, that is the [computed][] value of its parent and therefore also **not [important][]**.
The [computed][] value of the `letter-spacing` property of this `p` element is [not important](#24afc2:not-important). The [computed][] value of the `letter-spacing` property of this `span` element is the [inherited][] value, that is the [computed][] value of its parent and therefore also [not important](#24afc2:not-important).

```html
<p style="letter-spacing: 0.1em">
Expand All @@ -147,7 +151,7 @@ The [computed][] value of the `letter-spacing` property of this `p` element is *

#### Passed Example 8

The [computed][] value of the `letter-spacing` property of this `p` element is **not [important][]**. The [computed][] value of the `letter-spacing` property of this `span` element is the [inherited][] value, that is the [computed][] value of its parent and therefore also **not [important][]**.
The [computed][] value of the `letter-spacing` property of this `p` element is [not important](#24afc2:not-important). The [computed][] value of the `letter-spacing` property of this `span` element is the [inherited][] value, that is the [computed][] value of its parent and therefore also [not important](#24afc2:not-important).

```html
<p style="letter-spacing: 0.1em">
Expand All @@ -157,6 +161,50 @@ The [computed][] value of the `letter-spacing` property of this `p` element is *
</p>
```

#### Passed Example 9

This `div` element only has one text node descendant, and it is [wide enough](24afc2:wide-enough-cond) because its parent is the `p` element, with a `letter-spacing` of `2px`, more than `0.12` times the `font-size` of `10px`.

```html
<div style="font-size: 24px; letter-spacing: 2px !important">
<p style="font-size: 10px;">
The toy brought back fond memories of being lost in the rain forest.
</p>
</div>
```

#### Passed Example 10

This `div` element has no text node descendant and therefore matches the [spaced text](#24afc2:spaced-text).

```html
<div style="letter-spacing: 1px !important; border-top: 1px solid black;">
<!-- empty div, border to make it "visible" -->
</div>
```

#### Passed Example 11

This `div` element only has one text node descendant, and it is [empty](#24afc2:empty).

```html
<div style="letter-spacing: 1px !important; border-top: 1px solid black;">
&nbsp:<!-- empty div, border to make it "visible" -->
</div>
```

#### Passed Example 12

Both this `div` and `p` elements have a single text node descendant, whose parent (the `p` element) is [wide enough](#24afc2:wide-enough-cond)

```html
<div style="font-size: 24px; letter-spacing: 2px !important">
<p style="letter-spacing: 3px !important;">
The toy brought back fond memories of being lost in the rain forest.
</p>
</div>
```

### Failed

#### Failed Example 1
Expand Down Expand Up @@ -253,6 +301,7 @@ The `style` attribute of this `p` element does not [declare][declared] the `lett
[computed]: https://www.w3.org/TR/css-cascade-4/#computed 'CSS Cascading and Inheritance Level 4 (Working draft) - Computed Values'
[declared]: https://www.w3.org/TR/css-cascade-4/#declared 'CSS Cascading and Inheritance Level 4 (Working draft) - Declared Values'
[font-size]: https://www.w3.org/TR/css-fonts-4/#propdef-font-size 'CSS Fonts Module Level 4 (Working draft) - Font size: the font-size property'
[html element]: #namespaced-element
[important]: https://www.w3.org/TR/css-cascade-4/#importance 'CSS Cascading and Inheritance Level 4 (Working draft) - Importance'
[inherited]: https://www.w3.org/TR/css-cascade-4/#inheriting 'CSS Cascading and Inheritance Level 4 (Working draft) - Inherited Values'
[letter-spacing]: https://www.w3.org/TR/css-text-3/#propdef-letter-spacing 'CSS Text Module Level 3 - Tracking: the letter-spacing property'
Expand All @@ -263,4 +312,4 @@ The `style` attribute of this `p` element does not [declare][declared] the `lett
[user origin]: https://www.w3.org/TR/css-cascade-4/#cascade-origin-user 'CSS Cascading and Inheritance Level 4 (Working draft) - Cascading Origins - User Origin'
[user agent origin]: https://www.w3.org/TR/css-cascade-4/#cascade-origin-ua 'CSS Cascading and Inheritance Level 4 (Working draft) - Cascading Origins - User Agent Origin'
[visible]: #visible 'Definition of visible'
[html element]: #namespaced-element
[whitespace]: #whitespace 'Definition of whitespace'