Bug 239689

Summary:

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

Product:

WebKit

Reporter:

Chijin <tlock.chijin>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ahmad.saleem792, bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

Safari 15

Hardware:

All

OS:

All

Attachments:

Description	Flags
expected rendering result of chrome	none
Test reduction	none
Patch	none
Patch	none
Patch	none
[fast-cq]Patch	none

Description Chijin 2022-04-23 00:57:01 PDT

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.

Comment 1 zalan 2022-04-24 20:01:17 PDT

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).

Comment 2 zalan 2022-04-24 21:16:49 PDT

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.

Comment 3 Radar WebKit Bug Importer 2022-04-30 00:57:14 PDT

<rdar://problem/92559818>

Comment 4 Chijin 2022-05-02 19:50:46 PDT

Hi, anyone take care of this issue?

Comment 5 Ahmad Saleem 2022-12-31 04:16:50 PST

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

Comment 6 zalan 2022-12-31 07:16:20 PST

(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.

Comment 7 Chijin 2024-03-17 07:36:03 PDT

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

Comment 8 zalan 2024-03-17 13:55:51 PDT

Created attachment 470405 [details]
Patch

Comment 9 zalan 2024-03-17 13:56:40 PDT

(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.

Comment 10 zalan 2024-03-17 20:28:37 PDT

Created attachment 470413 [details]
Patch

Comment 11 Chijin 2024-03-18 00:08:06 PDT

Thanks for the fixing!

Comment 12 zalan 2024-03-18 05:32:18 PDT

Created attachment 470415 [details]
Patch

Comment 13 Antti Koivisto 2024-03-18 07:15:52 PDT

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.

Comment 14 zalan 2024-03-18 08:04:41 PDT

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

Comment 15 EWS 2024-03-18 09:45:36 PDT

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].

Comment 16 zalan 2024-03-18 10:00:58 PDT

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