58417 – SVG object covers CSS background in HTML foreignObject

RESOLVED FIXED 58417

SVG object covers CSS background in HTML foreignObject

https://bugs.webkit.org/show_bug.cgi?id=58417

Summary SVG object covers CSS background in HTML foreignObject

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	Details Formatted Diff Diff
Patch (5.88 KB, patch) 2011-11-08 10:49 PST, Florin Malita	no flags	Details Formatted Diff Diff
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.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware PC

OS Linux

Product WebKit

Component SVG

Assignee

Nobody

Reported

2011-04-13 01:37 PDT

Modified

2011-11-10 08:47 PST History

CC List

6 users Show

URL

Keywords

Depends on

Blocks