Bug 263698 – A floating element can cause the following element’s text-indent to fail

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Brad Andalman]

Created attachment 468341 [details]
HTML with paragraphs that should all display a text-indent

In the attached HTML, each paragraph should have a text-indent of 1.5em. 

However, in Safari 17.1 on Sonoma, the paragraph that follows the floating element (seen in lightgray) does not actually indent. Even when you change the floating element’s width to 89%, which pulls the first word of the following paragraph up, there is no text indent. You have to get to a width of 81% before the text-indent has any effect. This is not the case in Chrome and Firefox, where the text-indent always has an effect, regardless of the presence of a floating element immediately before the paragraph with the indent.

I can maybe understand why the text-indent doesn’t work for widths between 82-89%: there isn’t enough room for both the first word and the text indent. In that case, though, I’d argue that the word shouldn’t be brought up. In fact, that’s also what Chrome and Firefox do.

[Brad Andalman]

Created attachment 468342 [details]
Image showing rendered HTML in Safari, Chrome, Firefox

[Radar WebKit Bug Importer]

<rdar://problem/117538081>

[zalan]

Created attachment 468352 [details]
Test reduction

[zalan]

Thank you for reporting this issue and for the great test case!

[zalan]

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

[EWS]

Committed 269827@main (7137ef54ac3f): <https://commits.webkit.org/269827@main>

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

[WebKit Commit Bot]

Re-opened since this is blocked by bug 263773

[Brad Andalman]

Created attachment 468368 [details]
Additional test case

As long as this bug was reopened, I wanted to add an additional test case. 

Some background: I originally discovered this indentation bug in our application, which exhibits this problem with text indentation on Sonoma only. When coming up with a smaller test case, I discovered the test case I reported, which fails on all macOS versions I tried.

However, simply defining a column-width changes the behavior to match what I see in our application. Specifically, this new test case succeeds when you render it in Safari 16 (and earlier), but fails in Safari 17 (i.e. on Sonoma).

I’m hoping it’s the same underlying bug, so I’m attaching this new test case here. Please let me know if I should split it out into a new bug instead.

[zalan]

(In reply to Brad Andalman from comment #8)
> Created attachment 468368 [details]
> Additional test case
> 
> As long as this bug was reopened, I wanted to add an additional test case. 
> 
> Some background: I originally discovered this indentation bug in our
> application, which exhibits this problem with text indentation on Sonoma
> only. When coming up with a smaller test case, I discovered the test case I
> reported, which fails on all macOS versions I tried.
> 
> However, simply defining a column-width changes the behavior to match what I
> see in our application. Specifically, this new test case succeeds when you
> render it in Safari 16 (and earlier), but fails in Safari 17 (i.e. on
> Sonoma).
> 
> I’m hoping it’s the same underlying bug, so I’m attaching this new test case
> here. Please let me know if I should split it out into a new bug instead.
Thank you! This additional test case seems to be working on my local build (with the fix applied). I am going to re-test it once I resolved the perf regression but I encourage you to do the same once this re-lands on whatever type of build you have access to (local, STP, platform seed etc).
Thank you again!

[Brad Andalman]

That’s great news! Thanks, Zalan. 

I am new to this, so I don’t know the best way to test. I figured I could download the appropriate build from here:
https://webkit.org/build-archives/#mac-sonoma-x86_64%20arm64

But when I downloaded the build for 269827@main and ran run-webkit-archive, I didn’t see the fix when I loaded the test case in the Safari that launched. Do you have any advice for the best way to test? (Apologies for using this bug for this info…)

[zalan]

(In reply to Brad Andalman from comment #10)
> That’s great news! Thanks, Zalan. 
> 
> I am new to this, so I don’t know the best way to test. I figured I could
> download the appropriate build from here:
> https://webkit.org/build-archives/#mac-sonoma-x86_64%20arm64
> 
> But when I downloaded the build for 269827@main and ran run-webkit-archive,
> I didn’t see the fix when I loaded the test case in the Safari that
> launched. Do you have any advice for the best way to test? (Apologies for
> using this bug for this info…)
I tried run-webkit-archive and it failed to load the correct frameworks for Safari. Not sure what's going on here, maybe @ap has some insights?

[Alexey Proskuryakov]

I cannot reproduce this on a freshly installed macOS 14 (23A344) with SIP enabled, WebContent loads with downloaded frameworks as expected.

Brad, could you please file a bug at https://feedbackassistant.apple.com?
- Reproduce the issue, and note the time.
- File a report with a sysdiagnose attached, and with your steps to reproduce.
- Post the feedback number here.

Alan, since your situation may be different, please file a separate report, or I could take a look at your machine in person.

[Brad Andalman]

I’ve filed a bug using Feedback Assistant (FB13318964) as suggested in comment #12

[Alexey Proskuryakov]

Please do attach a sysdiagnose to FB13318964 (probably with a new timestamp, as original logs may be unavailable if you take the sysdiagnose now).

[Brad Andalman]

Sorry - I thought that was automatic. I’ve attached a sysdiagnose to that feedback.

[Alexey Proskuryakov]

I figured out why I couldn't reproduce. Will look into what can be done to make the archives easily usable, and in the meanwhile, I think that disabling SIP should help (https://developer.apple.com/documentation/security/disabling_and_enabling_system_integrity_protection).

This is probably enough of a digression, and the discussion can be turned back to the original layout issue. Please feel free to reach out to me in Slack or email if you'd like to discuss the problem with build archives.

[zalan]

Created attachment 468461 [details]
Patch

[zalan]

landing is pending on A/B perf test result.

[zalan]

(In reply to zalan from comment #18)
> landing is pending on A/B perf test result.
(which was supposedly fixed by https://commits.webkit.org/270075@main)

[zalan]

Created attachment 468485 [details]
Patch

[zalan]

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

[EWS]

Committed 270238@main (e918cedab35b): <https://commits.webkit.org/270238@main>

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