Bug 222323 - Fix model test differences between platforms
Summary: Fix model test differences between platforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-23 10:47 PST by Sam Weinig
Modified: 2021-02-24 12:12 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.65 KB, patch)
2021-02-23 11:20 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (3.07 KB, patch)
2021-02-23 11:22 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (1.30 KB, patch)
2021-02-24 08:45 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2021-02-23 10:47:10 PST
Fix model test differences between platforms - see https://bugs.webkit.org/show_bug.cgi?id=222114#c7.
Comment 1 Sam Weinig 2021-02-23 11:20:11 PST
Created attachment 421334 [details]
Patch
Comment 2 Sam Weinig 2021-02-23 11:22:13 PST
Created attachment 421335 [details]
Patch
Comment 3 Darin Adler 2021-02-23 13:05:06 PST
Comment on attachment 421335 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        Try to fix these on iOS by removing the doctype.

Wow, why does that matter?
Comment 4 EWS 2021-02-23 21:34:24 PST
Committed r273372: <https://commits.webkit.org/r273372>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421335 [details].
Comment 5 Radar WebKit Bug Importer 2021-02-23 21:35:17 PST
<rdar://problem/74678133>
Comment 6 Alexey Proskuryakov 2021-02-23 22:09:57 PST
I'm also very interested in an answer to Darin's question.
Comment 7 Sam Weinig 2021-02-24 08:44:27 PST
Re-opening, because the test is now skipped.
Comment 8 Sam Weinig 2021-02-24 08:45:55 PST
Created attachment 421413 [details]
Patch
Comment 9 Sam Weinig 2021-02-24 08:50:01 PST
(In reply to Darin Adler from comment #3)
> Comment on attachment 421335 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=421335&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        Try to fix these on iOS by removing the doctype.
> 
> Wow, why does that matter?

I'm not sure it does. Was putting the patch up to see. (I don't have a usable iOS currently to test it myself).

On slack, there was discussion about why iOS and macOS would have different render trees for this very simple test, and there wasn't a conclusive answer. Maybe something about rounding? I filed https://bugs.webkit.org/show_bug.cgi?id=222362 to track figuring it out.
Comment 10 Sam Weinig 2021-02-24 11:09:07 PST
Antti, Alan, Simon, did any of you have an explanation of why iOS and macOS are different in the end?
Comment 11 zalan 2021-02-24 11:10:14 PST
(In reply to Sam Weinig from comment #10)
> Antti, Alan, Simon, did any of you have an explanation of why iOS and macOS
> are different in the end?
I would need to debug this but I don't have a working iOS sim atm.
Comment 12 zalan 2021-02-24 11:18:22 PST
(In reply to Darin Adler from comment #3)
> Comment on attachment 421335 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=421335&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        Try to fix these on iOS by removing the doctype.
> 
> Wow, why does that matter?
Removing the doctype puts the document in non-standards mode which makes the descent on the line collapse.
It means that the line is only stretched by the replaced box and since the box sits on the baseline and has no descent, the line is going to be as tall as the replaced box is. 
In standards mode the line is also stretched by the root inline box which has the  default font's descent. (Now I just need to figure out if we produce different geometries on iOS because of some odd rounding mismatching or font differences)
Comment 13 EWS 2021-02-24 12:11:59 PST
Committed r273421: <https://commits.webkit.org/r273421>

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