Bug 233343 - Add a basic heuristic for sizing text in image overlay blocks
Summary: Add a basic heuristic for sizing text in image overlay blocks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
: 233111 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-11-18 16:24 PST by Wenson Hsieh
Modified: 2021-11-29 14:47 PST (History)
9 users (show)

See Also:


Attachments
Patch (14.58 KB, patch)
2021-11-18 18:38 PST, Wenson Hsieh
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Fix unused param (14.64 KB, patch)
2021-11-18 19:39 PST, Wenson Hsieh
thorton: review+
Details | Formatted Diff | Diff
Rebase on trunk (14.05 KB, patch)
2021-11-29 12:28 PST, Wenson Hsieh
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Fix unused param (14.09 KB, patch)
2021-11-29 12:59 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-11-18 16:24:07 PST
.
Comment 1 Wenson Hsieh 2021-11-18 18:38:04 PST Comment hidden (obsolete)
Comment 2 Wenson Hsieh 2021-11-18 19:39:07 PST
Created attachment 444773 [details]
Fix unused param
Comment 3 Radar WebKit Bug Importer 2021-11-25 16:25:23 PST
<rdar://problem/85755755>
Comment 4 Wenson Hsieh 2021-11-28 23:04:02 PST
*** Bug 233111 has been marked as a duplicate of this bug. ***
Comment 5 Tim Horton 2021-11-29 11:17:42 PST
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)
Comment 6 Wenson Hsieh 2021-11-29 12:18:34 PST
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.
Comment 7 Wenson Hsieh 2021-11-29 12:28:33 PST
Created attachment 445325 [details]
Rebase on trunk
Comment 8 Wenson Hsieh 2021-11-29 12:59:42 PST
Created attachment 445328 [details]
Fix unused param
Comment 9 EWS 2021-11-29 14:47:43 PST
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].