Bug 239689 – Setting white-space to non-default value dynamically on a blank (whitespace/new line) does not work correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chijin]

Created attachment 458204 [details]
expected rendering result of chrome

example url: https://codepen.io/chijinz/pen/eYyoLbz

Steps to reproduce the problem:

1. use Safari to visit the example url
2. the green light is on the top of the page even though white-space has been set to "pre-line". This is inconsistent with the document (https://developer.mozilla.org/en-US/docs/Web/CSS/white-space). Other browsers (e.g. chrome) works fine with this issue.

[zalan]

Created attachment 458239 [details]
Test reduction

This bug only occurs when the pre-line is set dynamically on an "inline empty" content (no text or other inline level box).

[zalan]

We don't create RenderText renderers for the whitespace/new line only content.

It looks like in RenderTreeUpdater::updateRenderTree
bool mayNeedUpdateWhitespaceOnlyRenderer = renderingParent().didCreateOrDestroyChildRenderer && text.data().isAllSpecialCharacters<isHTMLSpace>();
may need to be updated so that certain parent styles (e.g. white-space) are also taken into account when checking if the text node needs a renderer.

[Radar WebKit Bug Importer]

<rdar://problem/92559818>

[Chijin]

Hi, anyone take care of this issue?

[Ahmad Saleem]

I am still able to reproduce this in Safari 16.2 & STP160, is this something progressed with IFC on trunk? @Alan?

[zalan]

(In reply to zalan from comment #2)
> We don't create RenderText renderers for the whitespace/new line only
> content.
> 
> It looks like in RenderTreeUpdater::updateRenderTree
> bool mayNeedUpdateWhitespaceOnlyRenderer =
> renderingParent().didCreateOrDestroyChildRenderer &&
> text.data().isAllSpecialCharacters<isHTMLSpace>();
> may need to be updated so that certain parent styles (e.g. white-space) are
> also taken into account when checking if the text node needs a renderer.
Let's see what Antti thinks.

[Chijin]

Hi zalan, one year has been passed, any update for this bug?

[zalan]

Created attachment 470405 [details]
Patch

[zalan]

(In reply to Chijin from comment #7)
> Hi zalan, one year has been passed, any update for this bug?
This is not really my area, but let's see if I got this right.

[zalan]

Created attachment 470413 [details]
Patch

[Chijin]

Thanks for the fixing!

[zalan]

Created attachment 470415 [details]
Patch

[Antti Koivisto]

Comment on attachment 470415 [details]
Patch

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

> Source/WebCore/style/StyleTreeResolver.cpp:975
> +                if (!node.renderer() && containsOnlyASCIIWhitespace && parent.style.preserveNewline())
> +                    return true;

This could test !text->renderer() for consistency.
This really needs to be done only when parent.style.preserveNewline() changes value. We could set a bit in the parent struct when that happens and test for that.

[zalan]

Created attachment 470417 [details]
[fast-cq]Patch

[EWS]

Committed 276277@main (40f3c61c1d9d): <https://commits.webkit.org/276277@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 470417 [details].

[zalan]

(In reply to Chijin from comment #11)
> Thanks for the fixing!
Thank you for filing it!