Bug 14145 - RenderSVGContainer should not inherit from RenderContainer
Summary: RenderSVGContainer should not inherit from RenderContainer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-14 13:19 PDT by Eric Seidel (no email)
Modified: 2007-07-14 16:16 PDT (History)
0 users

See Also:


Attachments
partial fix (12.81 KB, patch)
2007-06-15 16:33 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
working patch (rob to finish) (18.65 KB, patch)
2007-06-18 09:59 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Initial patch (38.56 KB, patch)
2007-07-14 15:43 PDT, Nikolas Zimmermann
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-06-14 13:19:28 PDT
RenderSVGContainer should not inherit from RenderContainer

at least not now that RenderSVGRoot exists. :)

This may mean implementing a little bit of RenderContainer logic in RenderSVGContainer manually (like RenderContainer::paint() for instance.
Comment 1 Eric Seidel (no email) 2007-06-15 16:33:43 PDT
Created attachment 15070 [details]
partial fix

I'm not sure I'll have time/energy to finish this patch, but I thought I should post my partial work anyway.
Comment 2 Eric Seidel (no email) 2007-06-18 09:59:38 PDT
Created attachment 15101 [details]
working patch (rob to finish)

Ok, so this patch works completely (no crashes, etc).  There are two things which still need to be done.

1.  It breaks 4 test cases.  I believe this is something to do with hit-testing errors in RenderSVGContainer when it's being an <svg>
2.  Still need to change RenderSVGRoot::rendererName() to return RenderSVGRoot and update tests accordingly.
Comment 3 Nikolas Zimmermann 2007-07-14 15:43:32 PDT
Created attachment 15522 [details]
Initial patch

New patch incorporating Eric's patch, but bringing it to a working state w/o regressions.
Inlcudes some fixes from Rob made during WWDC.
Comment 4 Oliver Hunt 2007-07-14 15:47:54 PDT
Comment on attachment 15522 [details]
Initial patch

     // Only the root <svg> element should need any translations using the HTML/CSS system
     // parentX, parentY are also non-zero for first-level kids of these
     // CSS-transformed <svg> root-elements (due to RenderBox::paint) for any other element
     // they should be 0.   m_x, m_y should always be 0 for non-root svg containers
-    ASSERT(m_x == 0);
-    ASSERT(m_y == 0);

should the assertions not become isRoot || m_x == 0
etc?
Comment 5 Nikolas Zimmermann 2007-07-14 15:57:44 PDT
(In reply to comment #4)
> (From update of attachment 15522 [details] [edit])
>      // Only the root <svg> element should need any translations using the
> HTML/CSS system
>      // parentX, parentY are also non-zero for first-level kids of these
>      // CSS-transformed <svg> root-elements (due to RenderBox::paint) for any
> other element
>      // they should be 0.   m_x, m_y should always be 0 for non-root svg
> containers
> -    ASSERT(m_x == 0);
> -    ASSERT(m_y == 0);
> 
> should the assertions not become isRoot || m_x == 0
> etc?
> 

The outermost <svg> element is handled by RenderSVGRoot
nowadays. The commented is outdated, I'll fix it.

Greetings,
Niko
Comment 6 Nikolas Zimmermann 2007-07-14 16:16:50 PDT
Landed in r24292.