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
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
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
Note You need to log in before you can comment on or make changes to this bug.