Usage of textNode is overloaded

[shubhamg13]

Original Specifications

https://www.w3.org/TR/largest-contentful-paint/#compute-a-new-largest-contentful-paint-candidate

5. For each textNode of paintedTextNodes,

1. If textNode is not exposed for paint timing, given document, continue.

2. If textNode has alpha channel value <=0 or opacity value <=0:

1. If textNode’s text-shadow value is none, textNode’s stroke-color value is transparent and textNode’s stroke-image value is none, continue.

3. Let intersectionRect be the union of the border boxes of all Text nodes in textNode’s set of owned text nodes, intersected with the visual viewport.

List of References

From: https://www.w3.org/TR/paint-timing/#sec-recording-paint-timing

Each Element has a set of owned text nodes, which is an ordered set of Text nodes, initially empty. (b)

Each Document has a set of elements with rendered text, which is an ordered set of Elements, initially empty. (a)

From: https://www.w3.org/TR/paint-timing/#mark-paint-timing

4. Append element to paintedTextNodes. (c)

From: https://www.w3.org/TR/css-color-4/#the-color-property

Applies to:: all elements and text (d)
This property specifies the primary foreground color of the element. This is used as the fill color of its text content

From: https://www.w3.org/TR/css-text-decor-3/#text-shadow-property

This property accepts a comma-separated list of shadow effects to be applied to the text of the element. (e)

From: https://www.w3.org/TR/fill-stroke-3/#text-decor

For handling fills and strokes on text decorations, two sets of fill and stroke properties are defined (f)

[shubhamg13]

From above references, I can infer

1. Document has (set of) Elements <- from (a)
2. and Element has (set of) TextNodes <- from (b)
3. Element has color/alpha; that colors its text (not "textNode has alpha") <- from (d)
4. Also Containing Element has text-shadow (not textNode) <- from (e)

and

5. paintedTextNodes is container (maybe vector) of elements <- from (c)

Considering above inferences, I suggest following Correction to Step 5 of #compute-a-new-largest-contentful-paint-candidate

5. For each Element of paintedTextNodes,

1. If Element is not exposed for paint timing, given document, continue.

1. If Element has alpha channel value <=0 or opacity value <=0:

1. If Element’s text-shadow value is none, Element’s stroke-color value is transparent and Element’s stroke-image value is none, continue.

2. Let intersectionRect be the union of the border boxes of all Text nodes in Element’s set of owned text nodes, intersected with the visual viewport:

Extras

/* SCENARIO A: Text is still visible! (Outer condition matches, Inner condition FAILS) */
.outlined-text {
  color: transparent; /* alpha <= 0 (Outer condition = TRUE) */
  -webkit-text-stroke: 2px black; /* stroke-color is NOT transparent (Inner condition = FALSE) */
}
/* RESULT: The algorithm DOES NOT skip this. The text outline is an LCP candidate. */


/* SCENARIO B: Text is still visible! (Outer condition matches, Inner condition FAILS) */
.shadow-text {
  color: rgba(0, 0, 0, 0); /* alpha <= 0 (Outer condition = TRUE) */
  text-shadow: 2px 2px 4px black; /* text-shadow is NOT none (Inner condition = FALSE) */
}
/* RESULT: The algorithm DOES NOT skip this. The text shadow is an LCP candidate. */

[clelland]

The two changes here are -

• Replacing the variable name 'textNode' with the spec reference 'Element'
  • that's not correct; we're iterating over a set of Elements (that's how paintedTextNodes is defined), naming each of them in turn 'textNode'. The steps inside of the For each loop operate on individual Elements, not the concept of Element. (If this were an actual programming language, I'd say that it's confusing a variable name with a class name)
• Moving the visibility check into the union creation (or possibly the intersection?)
  • I'm not sure what the goal is here, and the word "considering" is unclear. In the initial algorithm, it's clear that if the Element named textNode isn't visible (fails any of the checks in 5.1 or 5.2.1) then we skip it and move on to the next Element. In your suggested text, it looks like maybe the visibility check is moved to each element of the set of owned text nodes, rather than being called on the owning element itself - is that accurate?

[shubhamg13]

Replacing the variable name 'textNode' with the spec reference 'Element'

that's not correct; we're iterating over a set of Elements (that's how paintedTextNodes is defined)

Refer (c)

Moving the visibility check into the union creation (or possibly the intersection?)

Yeah, you are right. Those are element properties.

---

I have updated the Description and Expectations.

Please take a look. If updated expectations looks reasonable.

OR

Very small and simple addition of following NOTE

NOTE: paintedTextNodes is a set of Elements. In this algorithm, textNode names each such Element (which has a set of owned text nodes), not a Text node.

[mmocny]

@shaseley and I will try to review and give feedback

[shaseley]

Re: paintedTextNodes, I'm not opposed to renaming it to paintedTextElements (and s/textNode/element) for clarity to differentiate between Text nodes -- but I don't think a note is necessary since it's declared as an ordered set of elements.

---

Re: opacity and other visibility checks -- @clelland: shouldn't those be on each individual text node and not the aggregating element? I'm thinking about the following case:

<div id="content">This is some <span style="opacity:0">very very very very very very long text</span</div>

IIUC "content" has two owned text nodes, but the opacity:0 only applies to one of them, and it doesn't apply to the aggregating element.

[shubhamg13]

Adding more references, In support of my reasoning.

From: https://w3c.github.io/element-timing/#sec-report-element-timing

For each Element element in paintedTextNodes:

From: https://w3c.github.io/element-timing/#sec-report-text

2. Let intersectionRect be an empty rectangle.
3. For each Text node text in element’s set of owned text nodes:

1. Augment intersectionRect to be smallest rectangle containing the border box of text and intersectionRect.

4. Intersect intersectionRect with the visual viewport.

[shubhamg13]

Also +1 to renaming to paintedTextElements

[shubhamg13]

Just found out #142 raises similar concern.