Bug 234745 – Fractional td width is not rendering correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: Tables

[Ismail Donmez]

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.

[Ismail Donmez]

Created attachment 448079 [details]
Safari 15.2 showing the wrong rendering

[Ismail Donmez]

Created attachment 448080 [details]
Firefox 95 showing the expected rendering

[Radar WebKit Bug Importer]

<rdar://problem/87162997>

[Ismail Donmez]

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.

[zalan]

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

[zalan]

Created attachment 449270 [details]
Patch

[Darin Adler]

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"?

[zalan]

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

[zalan]

Created attachment 449339 [details]
Patch

[EWS]

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