Bug 14145

Summary: RenderSVGContainer should not inherit from RenderContainer
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
partial fix
none
working patch (rob to finish)
none
Initial patch oliver: review+

Eric Seidel (no email)
Reported 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.
Attachments
partial fix (12.81 KB, patch)
2007-06-15 16:33 PDT, Eric Seidel (no email)
no flags
working patch (rob to finish) (18.65 KB, patch)
2007-06-18 09:59 PDT, Eric Seidel (no email)
no flags
Initial patch (38.56 KB, patch)
2007-07-14 15:43 PDT, Nikolas Zimmermann
oliver: review+
Eric Seidel (no email)
Comment 1 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.
Eric Seidel (no email)
Comment 2 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.
Nikolas Zimmermann
Comment 3 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.
Oliver Hunt
Comment 4 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?
Nikolas Zimmermann
Comment 5 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
Nikolas Zimmermann
Comment 6 2007-07-14 16:16:50 PDT
Landed in r24292.
Note You need to log in before you can comment on or make changes to this bug.