Bug 198763 – [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Wenson Hsieh]

<rdar://problem/51324136>

[Wenson Hsieh]

Created attachment 371964 [details]
WIP patch

[Wenson Hsieh]

<rdar://problem/51826266>

[Wenson Hsieh]

Created attachment 372807 [details]
Patch

[Wenson Hsieh]

Comment on attachment 372807 [details]
Patch

I’m going to see if I can make this a bit less crazy.

[Simon Fraser (smfr)]

Comment on attachment 372807 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyle.cpp:495
> +bool RenderStyle::isIdempotentTextAutosizingCandidate() const

You def. need some words about how this code came into being, and ideally you'd commit the tools used to generate this code, and the training dataset.

[Wenson Hsieh]

(In reply to Simon Fraser (smfr) from comment #5)
> Comment on attachment 372807 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372807&action=review
> 
> > Source/WebCore/rendering/style/RenderStyle.cpp:495
> > +bool RenderStyle::isIdempotentTextAutosizingCandidate() const
> 
> You def. need some words about how this code came into being,

Yes, will do! I put some words in the overall WebCore ChangeLog entry, but this definitely warrants more detail.

> and ideally
> you'd commit the tools used to generate this code, and the training dataset.

Interesting...into open source, that is? I think we’ll need to check if that’s kosher :/

[Simon Fraser (smfr)]

Maybe not, but to be maintainable, we have to be able to re-run your tools at any time.

[Wenson Hsieh]

(In reply to Simon Fraser (smfr) from comment #7)
> Maybe not, but to be maintainable, we have to be able to re-run your tools
> at any time.

Indeed. They are already accessible to all of us (WebKit/Safari at Apple), but I’ll work towards getting the tools into the main Safari internal repository.

[Wenson Hsieh]

Created attachment 372811 [details]
Less code, more words

[Simon Fraser (smfr)]

Comment on attachment 372811 [details]
Less code, more words

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

> Source/WebCore/ChangeLog:17
> +        Test: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-skip.html

Be careful with "skip" in a test name (I think we have a convention that filenames with "skipped" in them are skipped). I would call this something else.

> Source/WebCore/rendering/style/RenderStyle.cpp:497
> +    auto fields = OptionSet<AutosizeStatus::Fields>::fromRaw(m_inheritedFlags.autosizeStatus);

Need a comment at the top of this function referencing the radar that can point someone to the tools used to generate this code.

[Wenson Hsieh]

Created attachment 372817 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 372817 [details]
Patch for landing

Clearing flags on attachment: 372817

Committed r246781: <https://trac.webkit.org/changeset/246781>