Bug 233343 – Add a basic heuristic for sizing text in image overlay blocks

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Wenson Hsieh]

.

[Wenson Hsieh]

Created attachment 444769 [details]
Patch

[Wenson Hsieh]

Created attachment 444773 [details]
Fix unused param

[Radar WebKit Bug Importer]

<rdar://problem/85755755>

[Wenson Hsieh]

*** Bug 233111 has been marked as a duplicate of this bug. ***

[Tim Horton]

Comment on attachment 444773 [details]
Fix unused param

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

> Source/WebCore/dom/ImageOverlay.cpp:306
> +        rootContainer->setTranslate(false);

Seems like a nontrivial behavior change that should be in its own patch?

> Source/WebCore/dom/ImageOverlay.cpp:536
> +        document->updateLayoutIgnorePendingStylesheets();

Does the layout of the rest of the page affect these things at all? If they're independently positioned and sized, isn't this a bit overkill? (but also maybe necessary? I don't know)

[Wenson Hsieh]

Comment on attachment 444773 [details]
Fix unused param

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

Thanks for the review!

>> Source/WebCore/dom/ImageOverlay.cpp:306
>> +        rootContainer->setTranslate(false);
> 
> Seems like a nontrivial behavior change that should be in its own patch?

True! I will revert this change before landing, and file a separate bug for it.

>> Source/WebCore/dom/ImageOverlay.cpp:536
>> +        document->updateLayoutIgnorePendingStylesheets();
> 
> Does the layout of the rest of the page affect these things at all? If they're independently positioned and sized, isn't this a bit overkill? (but also maybe necessary? I don't know)

That's a good point — after chatting with Alan, it looks like each layout update here should be relatively cheap (assuming the rest of the document is laid out), since each of the boxes where we're changing font sizes are absolutely positioned.

I *think* we could use CSS containment to avoid part of the tree walk, but I think that cost is likely much less than that of running line breaking.

[Wenson Hsieh]

Created attachment 445325 [details]
Rebase on trunk

[Wenson Hsieh]

Created attachment 445328 [details]
Fix unused param

[EWS]

Committed r286265 (244628@main): <https://commits.webkit.org/244628@main>

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