Bug 58417

Summary: SVG object covers CSS background in HTML foreignObject
Product: WebKit Reporter: Raimund 'Raimi' Jacob <raimi>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: fmalita, hyatt, krit, simon.fraser, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
What FF4 does, correct.
none
What Chrome 11 Beta does, incorrect.
none
Testcase again as separate file
none
Same thing in plain SVG file
none
Patch
none
Patch none

Raimund 'Raimi' Jacob
Reported 2011-04-13 01:37:16 PDT
SVG object covers CSS background in HTML foreignObject The example shows an SVG inside a HTML page. The SVG in turn contains a group with a red 'rect' and a 'foreignObject'. The latter contains a HTML 'div' with a CSS class that defines bold font and yellow background. Everything is layered in a way that the foreignObject covers most of the red 'rect'. Expected behaviour: The HTML div is entirely visible and has a yellow background. Works in FF4. Actual behaviour: (Chrome 11 Beta, WebKit 534.24 (branches/chromium/696@83359)): The red 'rect' covers the yellow background of the HTML div. ---8<--- <html> <head> <title>SVG object covers CSS background in HTML foreignObject</title> <style> .My_Class { font-weight: bold; background: yellow; } </style> </head> <body> <div class="My_Class">Raimi was here</div> <svg id="svgWorld" xmlns="http://www.w3.org/2000/svg" width="500px" height="500px"> <g transform="translate(20, 20)"> <rect x="0" y="0" width="50" height="10" fill="red" /> <foreignObject x="5" y="1" width="200" height="20"><div class="My_Class">Raimi was here, too</div></foreignObject> </g> </svg> </body> </html>
Attachments
What FF4 does, correct. (3.20 KB, image/png)
2011-04-13 01:39 PDT, Raimund 'Raimi' Jacob 		no flags
Details
What Chrome 11 Beta does, incorrect. (9.77 KB, image/png)
2011-04-13 01:40 PDT, Raimund 'Raimi' Jacob 		no flags
Details
Testcase again as separate file (553 bytes, text/html)
2011-04-13 01:42 PDT, Raimund 'Raimi' Jacob 		no flags
Details
Same thing in plain SVG file (305 bytes, image/svg+xml)
2011-04-13 01:51 PDT, Raimund 'Raimi' Jacob 		no flags
Details
Patch (6.43 KB, patch)
2011-10-31 09:15 PDT, Florin Malita 		no flags
DetailsFormatted DiffDiff
Patch (5.88 KB, patch)
2011-11-08 10:49 PST, Florin Malita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Raimund 'Raimi' Jacob
Comment 1 2011-04-13 01:39:37 PDT
Created attachment 89349 [details] What FF4 does, correct.
Raimund 'Raimi' Jacob
Comment 2 2011-04-13 01:40:06 PDT
Created attachment 89350 [details] What Chrome 11 Beta does, incorrect.
Raimund 'Raimi' Jacob
Comment 3 2011-04-13 01:42:39 PDT
Created attachment 89351 [details] Testcase again as separate file
Raimund 'Raimi' Jacob
Comment 4 2011-04-13 01:51:54 PDT
Created attachment 89353 [details] Same thing in plain SVG file Demonstrates same issue, does not use CSS class but HTML style Attribute.
Raimund 'Raimi' Jacob
Comment 5 2011-05-06 05:07:30 PDT
PS: I believe this also interferes with (mouse) event handlers that are bound to the HTML-Div inside the foreignObject. They sometimes do not fire while over a "background-only" area. I'm not 100% sure - anyone interested in an extended testcase?
Florin Malita
Comment 6 2011-10-31 09:15:13 PDT
Created attachment 113057 [details] Patch
Nikolas Zimmermann
Comment 7 2011-10-31 09:22:04 PDT
Comment on attachment 113057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113057&action=review Looks good to me on first sight, but r- as it duplicates code. I wonder about the possible implications/side-effects though. I'd prefer if Dave/Simon/Dan could have a look! I'd want to hear their comments too. > Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:66 > + // Paint all phases of FO elements atomically, as though the FO element established its > + // own stacking context. Can't you share this code between InlineBox and RenderSVGFO in a common base? Replicating this is not a good idea.
Florin Malita
Comment 8 2011-10-31 10:56:01 PDT
Comment on attachment 113057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113057&action=review >> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:66 > > Can't you share this code between InlineBox and RenderSVGFO in a common base? > Replicating this is not a good idea. Sounds like a good idea, but there's a couple of things that make this tricky: * InlineBox & RenderSVGFO don't share a common base (InlineBox is a top level class) * while InlineBox::paint() just calls paint() on its RenderObject reference, RenderSVGFO::paint() delegates to a specific super method (RenderBlock::paint()) I'm not familiar enough with the code base to see a clean way around these - any suggestions?
Florin Malita
Comment 9 2011-11-08 10:49:47 PST
Created attachment 114116 [details] Patch
Florin Malita
Comment 10 2011-11-08 14:54:00 PST
Dave/Simon: do you mind taking a look at this? (not sure which Dan is Nikolas referring to, but hopefully I got the two of you right)
Simon Fraser (smfr)
Comment 11 2011-11-08 15:31:14 PST
Comment on attachment 114116 [details] Patch I think this a good change, but longer term maybe SVGFO needs it own RenderLayer which would do this.
Nikolas Zimmermann
Comment 12 2011-11-09 00:27:21 PST
(In reply to comment #11) > (From update of attachment 114116 [details]) > I think this a good change, but longer term maybe SVGFO needs it own RenderLayer which would do this. Great, thanks for looking quickly Simon. I've discussed this with Dirk some days ago. I still have an older patch lying around that adds a RenderLayer to RenderSVGFO/Root, but it's not complete at all.
Dirk Schulze
Comment 13 2011-11-09 02:41:37 PST
(In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 114116 [details] [details]) > > I think this a good change, but longer term maybe SVGFO needs it own RenderLayer which would do this. > Great, thanks for looking quickly Simon. I've discussed this with Dirk some days ago. > I still have an older patch lying around that adds a RenderLayer to RenderSVGFO/Root, but it's not complete at all. ...and I plan to continue working on it soon :)
Florin Malita
Comment 14 2011-11-10 07:19:08 PST
Is the plan still to commit this as a temporary fix then? I have another patch which depends on atomic FO painting to pass its tests, so I think that would make sense.
Nikolas Zimmermann
Comment 15 2011-11-10 07:33:06 PST
(In reply to comment #14) > Is the plan still to commit this as a temporary fix then? I have another patch which depends on atomic FO painting to pass its tests, so I think that would make sense. Yes, go ahead.
WebKit Review Bot
Comment 16 2011-11-10 07:55:12 PST
Comment on attachment 114116 [details] Patch Rejecting attachment 114116 [details] from commit-queue. schenney@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
WebKit Review Bot
Comment 17 2011-11-10 08:47:32 PST
Comment on attachment 114116 [details] Patch Clearing flags on attachment: 114116 Committed r99862: <http://trac.webkit.org/changeset/99862>
WebKit Review Bot
Comment 18 2011-11-10 08:47:37 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.