Bug 47053 – Rename (and move) RenderPath to svg/RenderSVGPath

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Andreas Kling]

RenderPath should be renamed to RenderSVGPath and moved from WebCore/rendering/ to WebCore/rendering/svg/

See bug 46051 for more information.

[Andreas Kling]

Created attachment 69579 [details]
Proposed patch

[Andreas Kling]

Comment on attachment 69579 [details]
Proposed patch

Caught a typo, new patch coming.

[Andreas Kling]

Created attachment 69580 [details]
Proposed patch v2

[Andreas Kling]

Note: The patch to rebaseline tests (needed due to renderName() change) is 19 MB, so I'm a little hesitant to upload it.

[Dirk Schulze]

Comment on attachment 69580 [details]
Proposed patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=69580&action=review

You can either land this patch together with your next patches or update the 19MB :-) r- for the wrong moving of the files + s/isVGPath/isRenderSVGPath/

> WebCore/ChangeLog:8
> +        Also renamed RenderObject::isRenderPath() to isSVGPath()

isRenderSVGPath(), should really be similar to the name of the renderer.

> WebCore/ChangeLog:22
> +        * rendering/RenderPath.cpp: Removed.
> +        * rendering/RenderPath.h: Removed.
> +        * rendering/RenderSVGHiddenContainer.cpp:

Please use 'svn mv' to move the files instead of copying. That is better for traversing  the history.

[Andreas Kling]

(In reply to comment #5)
> > WebCore/ChangeLog:8
> > +        Also renamed RenderObject::isRenderPath() to isSVGPath()
> 
> isRenderSVGPath(), should really be similar to the name of the renderer.

isSVGPath() is consistent with all the other RenderObject::isSVG*() methods.
 
> > WebCore/ChangeLog:22
> > +        * rendering/RenderPath.cpp: Removed.
> > +        * rendering/RenderPath.h: Removed.
> > +        * rendering/RenderSVGHiddenContainer.cpp:
> 
> Please use 'svn mv' to move the files instead of copying. That is better for traversing  the history.

D'oh, of course! I was using 'git mv' and didn't verify that prepare-ChangeLog (and friends) supported it.

[Dirk Schulze]

(In reply to comment #6)
> (In reply to comment #5)
> > > WebCore/ChangeLog:8
> > > +        Also renamed RenderObject::isRenderPath() to isSVGPath()
> > 
> > isRenderSVGPath(), should really be similar to the name of the renderer.
> 
> isSVGPath() is consistent with all the other RenderObject::isSVG*() methods.
> 
> > > WebCore/ChangeLog:22
> > > +        * rendering/RenderPath.cpp: Removed.
> > > +        * rendering/RenderPath.h: Removed.
> > > +        * rendering/RenderSVGHiddenContainer.cpp:
> > 
> > Please use 'svn mv' to move the files instead of copying. That is better for traversing  the history.
> 
> D'oh, of course! I was using 'git mv' and didn't verify that prepare-ChangeLog (and friends) supported it.

Hm, you're right, leave isSVGPath().

[Nikolas Zimmermann]

Patch looks great, please move everything with svn mv. 
I'm not here today, but Dirk may be able to review a revised version!

[Andreas Kling]

Created attachment 69604 [details]
Proposed patch v3

Same patch, but using 'svn mv' this time.

[Dirk Schulze]

(In reply to comment #9)
> Created an attachment (id=69604) [details]
> Proposed patch v3
> 
> Same patch, but using 'svn mv' this time.

Something looks wrong, the ChangeLog still says 'removed', and the RenderSVGPath files as well as the changes to these files are not in this patch.

[Early Warning System Bot]

Attachment 69604 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4160066

[Dirk Schulze]

Sorry, ChangeLog looks ok. But the changes to RenderPath are missing :-(

[Andreas Kling]

(In reply to comment #12)
> Sorry, ChangeLog looks ok. But the changes to RenderPath are missing :-(

Grahh, how do I convince svn to include the added file in the diff? It's marked "A" in "svn status"

[Eric Seidel (no email)]

Attachment 69604 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4184075

[WebKit Review Bot]

Attachment 69604 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4192072

[WebKit Review Bot]

Attachment 69604 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4149064

[Andreas Kling]

Created attachment 70036 [details]
Proposed patch v4

Let's hope the 4th time's the charm!

[Nikolas Zimmermann]

Comment on attachment 70036 [details]
Proposed patch v4

View in context: https://bugs.webkit.org/attachment.cgi?id=70036&action=review

> WebCore/WebCore.vcproj/WebCore.vcproj:33063
> +					RelativePath="..\rendering\svg\RenderSVGPath.cpp"

This is problematic, you're now excluding this file from the build.
If you do that, you have to add RenderSVGPath to rendering/RenderSVGAllInOne.cpp.
But as we didn't do that before, I'd say revert the FileConfiguration part.

r=me, if you fix that before landing, otherwhise win is broken.

[Andreas Kling]

Committed r69279: <http://trac.webkit.org/changeset/69279>