We should merge SVGLocatable and SVGTransformable into a new SVGGraphicsElement IDL interface, which inherits SVGElement, as per the latest specification: https://svgwg.org/svg2-draft/types.html#InterfaceSVGGraphicsElement This is one step towards getting rid of multiple inheritance in SVG, which is not longer supported by Web IDL. Corresponding Blink patch: https://src.chromium.org/viewvc/blink?revision=153166&view=revision
Created attachment 205694 [details] Patch
Comment on attachment 205694 [details] Patch Looks OK.
Comment on attachment 205694 [details] Patch Attachment 205694 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/974332
Created attachment 205697 [details] Attempt to fix mac-ews
Comment on attachment 205697 [details] Attempt to fix mac-ews Attachment 205697 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/966735
Created attachment 205704 [details] Another attempt to fix mac build
Comment on attachment 205704 [details] Another attempt to fix mac build Attachment 205704 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/981698
Created attachment 205707 [details] Another attempt to fix mac build
Created attachment 205711 [details] Patch
Kentaro, could you please take another quick look? I merged the fix from https://codereview.chromium.org/18054014/ (Moving SVGTests related code from SVGGraphics subclasses to SVGGraphics class) since this patch had not landed yet. Mac build should be OK now too.
Comment on attachment 205711 [details] Patch LGTM
(In reply to comment #11) > (From update of attachment 205711 [details]) > LGTM Could you please give some more hours for reviewers familiar with this code? This is was a simple renaming and cleanup. But follow up patches need a lot more attention and I would like have some one with a bit more experience to look over it before it gets reviewed - or at least committed. Thank you.
(In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 205711 [details] [details]) > > LGTM > > Could you please give some more hours for reviewers familiar with this code? This is was a simple renaming and cleanup. But follow up patches need a lot more attention and I would like have some one with a bit more experience to look over it before it gets reviewed - or at least committed. Thank you. Sure. Let me know when this can be committed or if you have comments.
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > (From update of attachment 205711 [details] [details] [details]) > > > LGTM > > > > Could you please give some more hours for reviewers familiar with this code? This is was a simple renaming and cleanup. But follow up patches need a lot more attention and I would like have some one with a bit more experience to look over it before it gets reviewed - or at least committed. Thank you. > > Sure. Let me know when this can be committed or if you have comments. Looked over it. Looks great for me. More worried when it comes to changing SVGLocatable elements to SVGGraphicsElements. This needs a lot of testing. Especially text related elements can be tricky.
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > (In reply to comment #11) > > > > (From update of attachment 205711 [details] [details] [details] [details]) > > > > LGTM > > > > > > Could you please give some more hours for reviewers familiar with this code? This is was a simple renaming and cleanup. But follow up patches need a lot more attention and I would like have some one with a bit more experience to look over it before it gets reviewed - or at least committed. Thank you. > > > > Sure. Let me know when this can be committed or if you have comments. > > Looked over it. Looks great for me. More worried when it comes to changing SVGLocatable elements to SVGGraphicsElements. This needs a lot of testing. Especially text related elements can be tricky. Ok, thanks for checking. I'll cc you from now on and give some time. I do agree that more complex patches are coming and help is welcome :)
Comment on attachment 205711 [details] Patch Clearing flags on attachment: 205711 Committed r152167: <http://trac.webkit.org/changeset/152167>
All reviewed patches have been landed. Closing bug.