Bug 234745

Summary: Fractional td width is not rendering correctly
Product: WebKit Reporter: Ismail Donmez <ismail>
Component: TablesAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, darin, esprehn+autocc, ews-watchlist, gyuyoung.kim, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Mac (Apple Silicon)   
OS: macOS 12   
Attachments:
Description Flags
Testcase
none
Safari 15.2 showing the wrong rendering
none
Firefox 95 showing the expected rendering
none
Patch
none
Patch none

Description Ismail Donmez 2021-12-29 13:54:10 PST
Created attachment 448078 [details]
Testcase

This is a copy of the Chrome bug I reported: https://bugs.chromium.org/p/chromium/issues/detail?id=1283025

Open the attached table.html testcase, and notice that even though the length of red should be in increasing order, we get 0.5% larger than even 5%. Attached also two screenshots, one of them showing the correct rendering in Firefox.
Comment 1 Ismail Donmez 2021-12-29 13:54:37 PST
Created attachment 448079 [details]
Safari 15.2 showing the wrong rendering
Comment 2 Ismail Donmez 2021-12-29 13:54:57 PST
Created attachment 448080 [details]
Firefox 95 showing the expected rendering
Comment 3 Radar WebKit Bug Importer 2022-01-05 13:55:32 PST
<rdar://problem/87162997>
Comment 4 Ismail Donmez 2022-01-15 03:14:53 PST
A fix is committed to Blink: https://chromium.googlesource.com/chromium/src/+/bfade5f8c943d322f5aca3ab0341824e4ae885a1

Adding here in the hopes that the code on WebKit side is also similar or the same.
Comment 5 zalan 2022-01-15 05:27:10 PST
(In reply to Ismail Donmez from comment #4)
> A fix is committed to Blink:
> https://chromium.googlesource.com/chromium/src/+/
> bfade5f8c943d322f5aca3ab0341824e4ae885a1
> 
> Adding here in the hopes that the code on WebKit side is also similar or the
> same.
The code is indeed similar! Thanks for following up on this.
Comment 6 zalan 2022-01-15 14:05:58 PST
Created attachment 449270 [details]
Patch
Comment 7 Darin Adler 2022-01-15 14:16:32 PST
Comment on attachment 449270 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        WinIE compatibility is probably no longer a valid reason to fail sizing table cells with width values between 0 and 1.

This is a misleading remark. The correct thing to say would be more like this:

    The code that ignores a width of zero was incorrectly parsing as an integer and ignoring trailing junk.
    That means that a width of "0.5" would be ignored by code that claimed it was ignoring width of zero.
    Seems unlikely that was ever desired, even "for compatibility with WinIE".

> Source/WebCore/html/HTMLTableCellElement.cpp:-106
> -        if (parseHTMLInteger(value).value_or(0) > 0) // width="0" is ignored for compatibility with WinIE.

This comment about width="0" shouldn’t necessarily be deleted. The new code still ignores widths of zero, doing it by passing AllowZeroValue::No, and it’s not obviously good to delete the comment that attempts to explain why.

But maybe the reason is now obvious? Maybe this is in the HTML or CSS specification now? Or maybe it’s still "for compatibility with WinIE"?
Comment 8 zalan 2022-01-15 14:29:43 PST
(In reply to Darin Adler from comment #7)
> Comment on attachment 449270 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449270&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        WinIE compatibility is probably no longer a valid reason to fail sizing table cells with width values between 0 and 1.
> 
> This is a misleading remark. The correct thing to say would be more like
> this:
> 
>     The code that ignores a width of zero was incorrectly parsing as an
> integer and ignoring trailing junk.
>     That means that a width of "0.5" would be ignored by code that claimed
> it was ignoring width of zero.
When I looked at the code this morning, I was like, look, it's parseHTMLInteger, it surely ignores all the fractional values and yet 4 hours later when I was writing the Changelog entry I completely failed to mention it :( 
Thanks for the review.
Comment 9 zalan 2022-01-17 09:23:42 PST
Created attachment 449339 [details]
Patch
Comment 10 EWS 2022-01-17 13:33:47 PST
Committed r288103 (246117@main): <https://commits.webkit.org/246117@main>

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