Bug 234235

Summary: [LBSE] Rename RenderSVGModelObject -> LegacyRenderSVGModelObject
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, rbuis, sabouhallawa, schenney, sergio, simon.fraser, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 90738, 234524    
Attachments:
Description Flags
Patch, v1
none
Patch, v2 none

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.