Bug 227033

Summary: Cleanup RenderElement::RendererCreationType
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, mifenton, pdr, simon.fraser, svillar, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

zalan
Reported 2021-06-15 10:01:34 PDT
it's confusing and highly error-prone.
Attachments
Patch (7.30 KB, patch)
2021-06-15 10:09 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (7.34 KB, patch)
2021-06-15 13:07 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (9.38 KB, patch)
2021-06-15 19:39 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
zalan
Comment 1 2021-06-15 10:09:21 PDT
Created attachment 431453 [details] Patch
zalan
Comment 2 2021-06-15 12:27:55 PDT
Looks like we've got one test covering this. Will adjust the patch.
zalan
Comment 3 2021-06-15 13:07:12 PDT
Created attachment 431471 [details] Patch
Simon Fraser (smfr)
Comment 4 2021-06-15 13:53:20 PDT
Comment on attachment 431471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431471&action=review > Source/WebCore/rendering/RenderElement.cpp:189 > + if (rendererTypeOverride.contains(ConstructBlockLevelRendererFor::TableTypes)) > return createRenderer<RenderBlockFlow>(element, WTFMove(style)); Is this entire 'default' block about table types? Seems like it would better to explicitly check for a table display type here?
zalan
Comment 5 2021-06-15 14:52:48 PDT
(In reply to Simon Fraser (smfr) from comment #4) > Comment on attachment 431471 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=431471&action=review > > > Source/WebCore/rendering/RenderElement.cpp:189 > > + if (rendererTypeOverride.contains(ConstructBlockLevelRendererFor::TableTypes)) > > return createRenderer<RenderBlockFlow>(element, WTFMove(style)); > > Is this entire 'default' block about table types? Seems like it would better > to explicitly check for a table display type here? Yeah, that's a good idea.
zalan
Comment 6 2021-06-15 19:39:29 PDT
Created attachment 431511 [details] Patch
EWS
Comment 7 2021-06-15 20:58:58 PDT
Committed r278919 (238850@main): <https://commits.webkit.org/238850@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431511 [details].
Radar WebKit Bug Importer
Comment 8 2021-06-15 20:59:17 PDT
<rdar://problem/79377750>
Note You need to log in before you can comment on or make changes to this bug.