WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
96804
Make computeLogicalHeight virtual and updateLogicalHeight non-virtual.
https://bugs.webkit.org/show_bug.cgi?id=96804
Summary
Make computeLogicalHeight virtual and updateLogicalHeight non-virtual.
Tony Chang
Reported
2012-09-14 10:17:33 PDT
In
bug 94982
, the code for computeLogicalHeight was made const and put int a non-virtual function. This got rid of some hacks where we would store the old height, call computeLogicalHeight, then restore the old height. However, in the new code, it's hard to use computeLogicalHeight because it's not virtual so if you call it on a pointer and your pointer overrides updateLogicalHeight (e.g., it's an iframe), it'll do the wrong thing. See RenderBlock::computeBlockPreferredLogicalWidths for an example of where we would like to use childBox->computeLogicalHeight(..), but it crashes because sometimes childBox is a RenderView, which overrides updateLogicalHeight(). I think long term, we want to make computeLogicalHeight virtual and move code from the subclass updateLogicalHeight into it and then make updateLogicalHeight non-virtual.
Attachments
Patch
(1.08 KB, patch)
2022-09-15 07:31 PDT
,
Rob Buis
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Tony Chang
Comment 1
2012-10-15 13:57:28 PDT
computeLogicalHeight is now virtual, however, I can't make updateLogicalHeight virtual because of RenderMultiColumnSet, which sets a member variable during updateLogicalHeight. It looks like that variable is the same as logicalHeight right now, but the comments suggest that there is a future use for it. I'd like to remove RenderMultiColumnSet::m_computedColumnHeight.
Dave Hyatt
Comment 2
2012-12-06 13:37:25 PST
Yeah, you're not going to be able to remove this. It's going to be needed.
Morten Stenshorne
Comment 3
2013-05-22 01:39:28 PDT
FYI: the RenderMultiColumnSet::updateLogicalHeight() override is going away with the fix for
https://bugs.webkit.org/show_bug.cgi?id=116033
, but in the meantime (after this bug was reported) RenderRegion::updateLogicalHeight() has appeared.
Rob Buis
Comment 4
2022-09-15 07:31:00 PDT
Created
attachment 462358
[details]
Patch
Tim Nguyen (:ntim)
Comment 5
2022-09-16 10:51:34 PDT
r=me
EWS
Comment 6
2022-09-16 11:58:03 PDT
Committed
254564@main
(c096c71bf576): <
https://commits.webkit.org/254564@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 462358
[details]
.
Radar WebKit Bug Importer
Comment 7
2022-09-16 11:59:19 PDT
<
rdar://problem/100036927
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug