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

[Jym77]

Currently updating only "Line height is not !important" to check whether that solution works or not.

The second and third condition where about the test target, which is incorrect since both the font-size or the text spacing property can be updated before the actual text. This replaces them with a condition that look at these properties on the parent of each text node inside the test target.

Closes issue(s):

• closes #1687
• closes #1688

Need for Call for Review: This will require a 2 weeks Call for Review (big change to expectation reworking the logic significantly)

---

Pull Request Etiquette

When creating PR:

• [X] Make sure you're requesting to pull a branch (right side) to the develop branch (left side).
• [X] Make sure you do not remove the "How to Review and Approve" section in your pull request description

After creating PR:

• [X] Add yourself (and co-authors) as "Assignees" for PR.
• [X] Add label to indicate if it's a Rule, Definition or Chore.
• [X] Link the PR to any issue it solves. This will be done automatically by referencing the issue at the top of this comment in the indicated place.
• [X] Optionally request feedback from anyone in particular by assigning them as "Reviewers".

When merging a PR:

• [ ] Close any issue that the PR resolves. This will happen automatically upon merging if the PR was correctly linked to the issue, e.g. by referencing the issue at the top of this comment.

How to Review And Approve

• Go to the “Files changed” tab
• Here you will have the option to leave comments on different lines.
• Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
• Make sure to also review the proposed Call for Review period. In case of disagreement, the longer period wins.

[Jym77]

This is getting really very complicated. I'm not sure this is actually necessary. To be honest I'd much rather we do this by just adding an assumption. Previously the rule of thumb has been that if we have no real-world examples of a problem, we're can use those. WDYT?

I'm not sure how much of an actual problem the full rule detects in the first place 😅

Thus said, I feel the way to simplicity would be to move focus of the rule from the element to the text node (so from where the problem is created to where it actually occurs):

Applicability: any (parent of a) (non-whitespace) text node.

Expectation: either (i) specified letter-spacing is not declared in a style attribute; or (ii) specified letter-spacing is not !important; or (iii) computed letter-spacing is larger than X times computed font-size.

This immediately rules out the corner cases where a definition is not affecting any text, and freely handle the cases where the properties get changed. Maybe that would need to deprecate these rules and create new ones since it is a significant change 🤔

@WilcoFiers what do you think about this?

[WilcoFiers]

@Jym77 Just for how much that changes these rules, I'd rather not. If we wanted to fix this (and I'm not convinced we need to) perhaps the way to do it would be by doing this:

- **wide enough**: the [computed][] [letter-spacing][] is at least 0.12 times the [computed][] [font-size][] of all descending text node in the flat tree; or

[Jym77]

@Jym77 Just for how much that changes these rules, I'd rather not. If we wanted to fix this (and I'm not convinced we need to) perhaps the way to do it would be by doing this:

- **wide enough**: the [computed][] [letter-spacing][] is at least 0.12 times the [computed][] [font-size][] of all descending text node in the flat tree; or

That fixes #1688 (font-size changes), but not #1687 (letter-spacing change) where in the second example the div would still incorrectly fail the rule.

[dd8]

How about wording like this:

Applicability

Elements where the computed value of the letter-spacing: property was assigned from a letter-spacing: declared as !important in a style attribute.

Expectation

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

• only whitespace: the element contains only whitespace text nodes; or
• wide enough: the element has wide enough letter spacing

The avoids having to embed details of how the CSS the cascade works in the rule, and lets implementors choose how they implement this (looking at cascaded value, looking at specificity, calling an API, etc)

The definition above seems to work for all these cases:

<div style="letter-spacing: 2.4px !important; font-size: 30px">
  <span>&nbsp;</span> <!-- OK because too small but only whitespace -->
  <span style="font-size: 20px">Lorem</span> <!-- OK letter-spacing assigned from div, but font-size is smaller -->
  <span style="letter-spacing: 3.6px !important">ipsum <!-- OK letter-spacing assigned from span --></span> 
  <span style="letter-spacing: 4.6px">ipsum <!-- OK letter-spacing assigned from span, and doesn't inherit from div --></span> 
  <span>dolor sit amet </span> <!-- Bad: letter-spacing assigned from div via implicit inheritance -->
  <span style="letter-spacing: inherit">ipsum <!-- Bad: letter-spacing assigned from div via explicit inheritance --></span> 
  
 Also bad <!-- Bad: letter-spacing assigned from div -->
</div>

[Jym77]

How about wording like this:

Applicability

Elements where the computed value of the letter-spacing: property was assigned from a letter-spacing: declared as !important in a style attribute.

I'm not sure what "the computed value was assigned from" means. I guess i will boil down to something like "the specified value was declared in" (an !important style attribute) 🤔 In any case we'll need to define it since it doesn't refer to any existing term as far as I can tell.

[dd8]

assign comes from here:

Once a user agent has parsed a document and constructed a document tree, it must assign, to every element in the tree, and correspondingly to every box in the formatting structure, a value to every property that applies to the target media type

https://www.w3.org/TR/css-cascade-3/#value-stages

Here's a tweaked version with a link to the definition:

Applicability

Elements where the computed value of the letter-spacing: property was assigned from a letter-spacing: declaration marked as as !important in an HTML style attribute.

[Jym77]

Closed by #1923