Bug 234235 - [LBSE] Rename RenderSVGModelObject -> LegacyRenderSVGModelObject
Summary: [LBSE] Rename RenderSVGModelObject -> LegacyRenderSVGModelObject
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords: InRadar
Depends on:
Blocks: 90738 234524
  Show dependency treegraph
 
Reported: 2021-12-13 05:08 PST by Nikolas Zimmermann
Modified: 2022-01-05 04:42 PST (History)
19 users (show)

See Also:


Attachments
Patch, v1 (50.16 KB, patch)
2021-12-13 05:15 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch, v2 (37.73 KB, patch)
2021-12-21 12:40 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 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.
Comment 1 Nikolas Zimmermann 2021-12-13 05:15:50 PST
Created attachment 446990 [details]
Patch, v1
Comment 2 Rob Buis 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?
Comment 3 Nikolas Zimmermann 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?
Comment 4 Rob Buis 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!
Comment 5 Nikolas Zimmermann 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.
Comment 6 Nikolas Zimmermann 2021-12-13 11:55:32 PST
Committed r286962 (245185@trunk): <https://commits.webkit.org/245185@trunk>
Comment 7 Radar WebKit Bug Importer 2021-12-13 11:56:21 PST
<rdar://problem/86425263>
Comment 8 Nikolas Zimmermann 2021-12-21 12:40:15 PST
Reopening to attach new patch.
Comment 9 Nikolas Zimmermann 2021-12-21 12:40:20 PST
Created attachment 447744 [details]
Patch, v2
Comment 10 Nikolas Zimmermann 2021-12-21 12:42:11 PST
Sorry for the noise, typo in the bug number... no new patch is needed here.