Bug 233343

Summary:

Add a basic heuristic for sizing text in image overlay blocks

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

Platform

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

akeerthi, cdumez, esprehn+autocc, ews-watchlist, hi, kangil.han, megan_gardner, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	ews-feeder: commit-queue-
Fix unused param	thorton: review+
Rebase on trunk	ews-feeder: commit-queue-
Fix unused param	none

Wenson Hsieh

Reported 2021-11-18 16:24:07 PST

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2021-11-18 18:38:04 PST Comment hidden (obsolete)

Created attachment 444769 [details] Patch

Wenson Hsieh

Comment 2 2021-11-18 19:39:07 PST

Created attachment 444773 [details] Fix unused param

Radar WebKit Bug Importer

Comment 3 2021-11-25 16:25:23 PST

<rdar://problem/85755755>

Wenson Hsieh

Comment 4 2021-11-28 23:04:02 PST

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

Tim Horton

Comment 5 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)

Wenson Hsieh

Comment 6 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.

Wenson Hsieh

Comment 7 2021-11-29 12:28:33 PST

Created attachment 445325 [details] Rebase on trunk

Wenson Hsieh

Comment 8 2021-11-29 12:59:42 PST

Created attachment 445328 [details] Fix unused param

EWS

Comment 9 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].

Note You need to log in before you can comment on or make changes to this bug.