RESOLVED FIXED 234235
[LBSE] Rename RenderSVGModelObject -> LegacyRenderSVGModelObject
https://bugs.webkit.org/show_bug.cgi?id=234235
Summary [LBSE] Rename RenderSVGModelObject -> LegacyRenderSVGModelObject
Nikolas Zimmermann
Reported 2021-12-13 05:08:56 PST
The LBSE RenderSVGModelObject will inherit from RenderLayerModelObject. Therefore we have to re-introduce a fresh RenderSVGModelObject for LBSE and rename the current one.
Attachments
Patch, v1 (50.16 KB, patch)
2021-12-13 05:15 PST, Nikolas Zimmermann
no flags
Patch, v2 (37.73 KB, patch)
2021-12-21 12:40 PST, Nikolas Zimmermann
no flags
Nikolas Zimmermann
Comment 1 2021-12-13 05:15:50 PST
Created attachment 446990 [details] Patch, v1
Rob Buis
Comment 2 2021-12-13 06:27:43 PST
Comment on attachment 446990 [details] Patch, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=446990&action=review > Source/WebCore/rendering/RenderObject.h:351 > + bool isRenderSVGModelObjectOrLegacyRenderSVGModelObject() const { return isRenderSVGModelObject() || isLegacyRenderSVGModelObject(); } Not too fond of the length. isARenderSVGModelObject? isRenderSVGModelObjectLike?
Nikolas Zimmermann
Comment 3 2021-12-13 10:38:28 PST
(In reply to Rob Buis from comment #2) > Comment on attachment 446990 [details] > Patch, v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=446990&action=review > > > Source/WebCore/rendering/RenderObject.h:351 > > + bool isRenderSVGModelObjectOrLegacyRenderSVGModelObject() const { return isRenderSVGModelObject() || isLegacyRenderSVGModelObject(); } > > Not too fond of the length. isARenderSVGModelObject? > isRenderSVGModelObjectLike? I named it following the convention for RenderSVGRoot, which established isRenderSVGRootOrLegacySVGRoot. I think if we change this, we should change both, no? isRenderOrLegacyRenderSVGModelObject / isRenderOrLegacyRenderSVGRoot Is that better?
Rob Buis
Comment 4 2021-12-13 10:48:27 PST
Comment on attachment 446990 [details] Patch, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=446990&action=review >>> Source/WebCore/rendering/RenderObject.h:351 >>> + bool isRenderSVGModelObjectOrLegacyRenderSVGModelObject() const { return isRenderSVGModelObject() || isLegacyRenderSVGModelObject(); } >> >> Not too fond of the length. isARenderSVGModelObject? isRenderSVGModelObjectLike? > > I named it following the convention for RenderSVGRoot, which established isRenderSVGRootOrLegacySVGRoot. I think if we change this, we should change both, no? > > isRenderOrLegacyRenderSVGModelObject / isRenderOrLegacyRenderSVGRoot > Is that better? Yes, I like that better!
Nikolas Zimmermann
Comment 5 2021-12-13 11:28:57 PST
(In reply to Rob Buis from comment #4) > Comment on attachment 446990 [details] > Patch, v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=446990&action=review > > >>> Source/WebCore/rendering/RenderObject.h:351 > >>> + bool isRenderSVGModelObjectOrLegacyRenderSVGModelObject() const { return isRenderSVGModelObject() || isLegacyRenderSVGModelObject(); } > >> > >> Not too fond of the length. isARenderSVGModelObject? isRenderSVGModelObjectLike? > > > > I named it following the convention for RenderSVGRoot, which established isRenderSVGRootOrLegacySVGRoot. I think if we change this, we should change both, no? > > > > isRenderOrLegacyRenderSVGModelObject / isRenderOrLegacyRenderSVGRoot > > Is that better? > > Yes, I like that better! Good will land that with the new name. One thing was incorrect: the naming scheme for RenderSVGRoot is different, the identifier is not named isRenderSVGRoot() but isSVGRoot(), thus a different naming scheme is fine for RenderSVGRoot - it can stay as-is.
Nikolas Zimmermann
Comment 6 2021-12-13 11:55:32 PST
Radar WebKit Bug Importer
Comment 7 2021-12-13 11:56:21 PST
Nikolas Zimmermann
Comment 8 2021-12-21 12:40:15 PST
Reopening to attach new patch.
Nikolas Zimmermann
Comment 9 2021-12-21 12:40:20 PST
Created attachment 447744 [details] Patch, v2
Nikolas Zimmermann
Comment 10 2021-12-21 12:42:11 PST
Sorry for the noise, typo in the bug number... no new patch is needed here.
Note You need to log in before you can comment on or make changes to this bug.