Bug 247196 – [content-visibility] Make innerText take content-visibility: hidden into account

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Rob Buis]

[content-visibility] Make innerText take css content-visibility: hidden into account.

[Rob Buis]

Created attachment 463295 [details]
Patch

[Rob Buis]

Created attachment 463349 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/101959795>

[Rob Buis]

Created attachment 463808 [details]
Patch

[Rob Buis]

Created attachment 464074 [details]
Patch

[Tim Nguyen (:ntim)]

Comment on attachment 464074 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464074&action=review

> Source/WebCore/editing/TextIterator.cpp:487
> +            } else if (!isSkippedText(*renderer)) {

Isn't this just the same as `else {`? Can you even set `content-visibility: hidden` on a text node?

[Ryosuke Niwa]

Comment on attachment 464074 [details]
Patch

What happens if content-visibility: visible content appears within content-visibility: hidden. Would it also be hidden?

[Tim Nguyen (:ntim)]

(In reply to Ryosuke Niwa from comment #7)
> Comment on attachment 464074 [details]
> Patch
> 
> What happens if content-visibility: visible content appears within
> content-visibility: hidden. Would it also be hidden?

The spec says `content-visibility: hidden` is similar to `display: none` so I would assume it would be skipped.

https://w3c.github.io/csswg-drafts/css-contain/#content-visibility

Rob would know better than I would though.

[Rob Buis]

(In reply to Tim Nguyen (:ntim) from comment #8)
> (In reply to Ryosuke Niwa from comment #7)
> > Comment on attachment 464074 [details]
> > Patch
> > 
> > What happens if content-visibility: visible content appears within
> > content-visibility: hidden. Would it also be hidden?
> 
> The spec says `content-visibility: hidden` is similar to `display: none` so
> I would assume it would be skipped.
> 
> https://w3c.github.io/csswg-drafts/css-contain/#content-visibility
> 
> Rob would know better than I would though.

Ryosuke is correct, content-visibility: hidden overrides any value of content-visibility in its subtree (also note that 'visible' is the default for this property).

[Rob Buis]

Comment on attachment 464074 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=464074&action=review

>> Source/WebCore/editing/TextIterator.cpp:487
>> +            } else if (!isSkippedText(*renderer)) {
> 
> Isn't this just the same as `else {`? Can you even set `content-visibility: hidden` on a text node?

Not directly, but RenderText style goes through the parent style, so it is valid to test for effectiveSkipsContent. I need to do more testing with this patch though to see how much of this patch is actually needed. I am also wary of the effect this has on content-visibility WPT tests and it rendering some text as blank and giving output that gives no clue to the issuer. To that end I am fixing those tests (like https://github.com/web-platform-tests/wpt/pull/37552) but I have to see how many need to be adjusted.

[Rob Buis]

Pull request: https://github.com/WebKit/WebKit/pull/7828

[EWS]

Committed 258187@main (63fdf6b4ce81): <https://commits.webkit.org/258187@main>

Reviewed commits have been landed. Closing PR #7828 and removing active labels.