Bug 47053 - Rename (and move) RenderPath to svg/RenderSVGPath
Summary: Rename (and move) RenderPath to svg/RenderSVGPath
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 46051
  Show dependency treegraph
 
Reported: 2010-10-02 13:46 PDT by Andreas Kling
Modified: 2010-10-07 00:33 PDT (History)
9 users (show)

See Also:


Attachments
Proposed patch (80.76 KB, patch)
2010-10-02 13:53 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Proposed patch v2 (80.80 KB, patch)
2010-10-02 14:16 PDT, Andreas Kling
krit: review-
Details | Formatted Diff | Diff
Proposed patch v3 (59.23 KB, patch)
2010-10-04 01:21 PDT, Andreas Kling
kling: review-
Details | Formatted Diff | Diff
Proposed patch v4 (83.59 KB, patch)
2010-10-06 22:15 PDT, Andreas Kling
zimmermann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2010-10-02 13:46:04 PDT
RenderPath should be renamed to RenderSVGPath and moved from WebCore/rendering/ to WebCore/rendering/svg/

See bug 46051 for more information.
Comment 1 Andreas Kling 2010-10-02 13:53:24 PDT
Created attachment 69579 [details]
Proposed patch
Comment 2 Andreas Kling 2010-10-02 14:02:44 PDT
Comment on attachment 69579 [details]
Proposed patch

Caught a typo, new patch coming.
Comment 3 Andreas Kling 2010-10-02 14:16:55 PDT
Created attachment 69580 [details]
Proposed patch v2
Comment 4 Andreas Kling 2010-10-02 14:42:16 PDT
Note: The patch to rebaseline tests (needed due to renderName() change) is 19 MB, so I'm a little hesitant to upload it.
Comment 5 Dirk Schulze 2010-10-02 22:15:19 PDT
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.
Comment 6 Andreas Kling 2010-10-02 22:23:38 PDT
(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.
Comment 7 Dirk Schulze 2010-10-02 22:57:19 PDT
(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().
Comment 8 Nikolas Zimmermann 2010-10-03 02:58:01 PDT
Patch looks great, please move everything with svn mv. 
I'm not here today, but Dirk may be able to review a revised version!
Comment 9 Andreas Kling 2010-10-04 01:21:40 PDT
Created attachment 69604 [details]
Proposed patch v3

Same patch, but using 'svn mv' this time.
Comment 10 Dirk Schulze 2010-10-04 01:29:48 PDT
(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.
Comment 11 Early Warning System Bot 2010-10-04 01:35:41 PDT
Attachment 69604 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4160066
Comment 12 Dirk Schulze 2010-10-04 01:37:48 PDT
Sorry, ChangeLog looks ok. But the changes to RenderPath are missing :-(
Comment 13 Andreas Kling 2010-10-04 01:39:50 PDT
(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"
Comment 14 Eric Seidel (no email) 2010-10-04 01:51:35 PDT
Attachment 69604 [details] did not build on mac:
Build output: http://queues.webkit.org/results/4184075
Comment 15 WebKit Review Bot 2010-10-04 02:14:58 PDT
Attachment 69604 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4192072
Comment 16 WebKit Review Bot 2010-10-04 02:24:15 PDT
Attachment 69604 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4149064
Comment 17 Andreas Kling 2010-10-06 22:15:22 PDT
Created attachment 70036 [details]
Proposed patch v4

Let's hope the 4th time's the charm!
Comment 18 Nikolas Zimmermann 2010-10-07 00:11:22 PDT
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.
Comment 19 Andreas Kling 2010-10-07 00:33:58 PDT
Committed r69279: <http://trac.webkit.org/changeset/69279>