Bug 209669 – [AutoSizing] Bring back the old auto-sizing code as a deprecated codepath for compatibility reasons

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Antoine Quint]

[AutoSizing] Bring back the old auto-sizing code as a deprecated codepath for compatibility reasons

[Antoine Quint]

Created attachment 394735 [details]
Patch

[zalan]

Comment on attachment 394735 [details]
Patch

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

> Source/WebCore/page/FrameViewLayoutContext.cpp:236
>          view().autoSizeIfEnabled();
> +        view().legacyAutoSizeIfEnabled();

Can we instead move it to autoSizeIfEnabled and branch in there for legacy behavior?

[Simon Fraser (smfr)]

Please include the radar.

[Simon Fraser (smfr)]

Can we also rename to avoid confusion with text autosizing.

[Antoine Quint]

Created attachment 394750 [details]
Patch

[zalan]

FrameView&co looks good!

[Antoine Quint]

Created attachment 394799 [details]
Patch

[Antoine Quint]

Created attachment 394803 [details]
Patch

[Antoine Quint]

Created attachment 394809 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 394809 [details]
Patch

Can we call this something other than "deprecated" auto-size mode? How does it differ from the non-deprecated one?

[zalan]

(In reply to Simon Fraser (smfr) from comment #10)
> Comment on attachment 394809 [details]
> Patch
> 
> Can we call this something other than "deprecated" auto-size mode? How does
> it differ from the non-deprecated one?
It's very different. The old auto-size flattens the content, the new one does not. The current auto-size code pretty much just lays out the content with a fixed constraint and reports the intrinsic size.

[Darin Adler]

Comment on attachment 394809 [details]
Patch

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

This looks OK but I’m very surprised that we are not adding any tests for this

> Source/WebCore/page/FrameView.h:710
> +    enum class AutoSizeMode : uint8_t { Default, Deprecated };

Could use bool instead of uint8_t. Do we know if one is superior?

[Antoine Quint]

Created attachment 394870 [details]
Patch

[Antoine Quint]

Created attachment 394872 [details]
Patch

[Antoine Quint]

Created attachment 394874 [details]
Patch

[Antoine Quint]

Created attachment 394876 [details]
Patch

[Antoine Quint]

https://trac.webkit.org/changeset/259187/webkit

[Radar WebKit Bug Importer]

<rdar://problem/61044059>

[Antoine Quint]

<rdar://problem/60111081>

[Timothy Hatcher]

Comment on attachment 394876 [details]
Patch

Clearing review flag. This landed.