Bug 10687 - object embedded in HTML: default background should be transparent.
Summary: object embedded in HTML: default background should be transparent.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 420+
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.peepo.com
Keywords: Regression
: 17744 18964 27078 37416 48462 (view as bug list)
Depends on:
Blocks: 48462
  Show dependency treegraph
 
Reported: 2006-09-02 01:27 PDT by jay
Modified: 2011-01-27 11:31 PST (History)
19 users (show)

See Also:


Attachments
a circle with no background defined (275 bytes, image/svg+xml)
2006-09-02 01:32 PDT, jay
no flags Details
html background with 'default' white svg background demonstrated (516 bytes, text/html)
2006-09-02 01:40 PDT, jay
no flags Details
html background with 'default' white svg background demonstrated (535 bytes, text/html)
2010-01-04 03:31 PST, jay
no flags Details
Reduced one-file test case using data: URL, no white should be visible (638 bytes, text/html)
2010-09-20 08:00 PDT, Jeff Schiller
no flags Details
Patch to make SVG documents embedded using <object>, <embed> have transparent backgrounds. (786 bytes, patch)
2010-09-20 09:22 PDT, Jeff Schiller
krit: review-
Details | Formatted Diff | Diff
This patch makes all backgrounds of SVG files served by reference in object, embed and iframes transparent. (8.43 KB, patch)
2010-09-20 20:44 PDT, Jeff Schiller
hyatt: review-
Details | Formatted Diff | Diff
Test for text/xml documents with a <svg> root (5.12 KB, text/xml)
2010-09-21 07:26 PDT, Jeff Schiller
no flags Details
Patch for making SVG documents and XML documents with SVG root transparent when embedded by reference. (11.29 KB, patch)
2010-09-23 06:50 PDT, Jeff Schiller
hyatt: review-
Details | Formatted Diff | Diff
Patch addressing dhyatt's comments (no caching of hasSVGRootNode). (10.28 KB, patch)
2010-09-23 12:00 PDT, Jeff Schiller
hyatt: review+
Details | Formatted Diff | Diff
Patch with webkit style fixups. (10.28 KB, patch)
2010-09-23 12:17 PDT, Jeff Schiller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jay 2006-09-02 01:27:53 PDT
currently SVGs embedded in HTML have a white background
Comment 1 jay 2006-09-02 01:32:45 PDT
Created attachment 10359 [details]
a circle with no background defined
Comment 2 jay 2006-09-02 01:40:57 PDT
Created attachment 10360 [details]
html background with 'default' white svg background demonstrated
Comment 3 jay 2006-09-02 01:43:52 PDT
SVG background should be transparent in default
Comment 4 Eric Seidel (no email) 2006-09-02 01:46:25 PDT
The spec agrees here:
http://www.w3.org/TR/SVG/render.html#ParentCompositing
Comment 5 Eric Seidel (no email) 2006-09-02 02:19:54 PDT
Actually, I think this might be the default behavior of <object>, not of the svg which is causing the white background.
Comment 6 jay 2006-09-02 08:09:29 PDT
#5

Eric, sorry I suggested similar via IRC not the bug.

should we widen scope or where should we file the bug elsewhere?

cheers
Comment 7 jay 2006-09-03 09:50:21 PDT
moving to new bugs and widening scope to object from svg
Comment 8 jay 2006-09-03 10:10:06 PDT
following comment from MacDome:

firefox and opera give transparent backgrounds.

Comment 9 jay 2008-09-04 13:42:16 PDT
regression as reported in another bug
Comment 10 jay 2008-09-04 13:45:18 PDT
the later bug is #18964
Comment 11 Cameron McCormack (:heycam) 2009-07-15 17:40:29 PDT
*** Bug 18964 has been marked as a duplicate of this bug. ***
Comment 12 Cameron McCormack (:heycam) 2009-07-15 17:41:00 PDT
*** Bug 27078 has been marked as a duplicate of this bug. ***
Comment 13 Cameron McCormack (:heycam) 2009-07-15 17:42:02 PDT
*** Bug 17744 has been marked as a duplicate of this bug. ***
Comment 14 CPK Smithies 2009-07-15 21:59:58 PDT
I have observed this with chromium-browser under Linux also. It doesn't seem to occur under Safari/Chrome on Windows.
Comment 15 Jonathan Leech 2009-09-25 09:37:37 PDT
The regression appears to be complete - confirmed in Google Chrome 3.0.195.21 and Safari for Windows 4.0.2 (530.19.1).
Comment 16 Matthieu Chaton 2009-10-19 03:37:37 PDT
I think the title is false, because it's not just a problem of default background. Even if I set a background color to my svg object, the background is still white (where in firefox 3.5 the background color is applied)

I have a xhtml page, with the svg included this way :
<object id='module_txt_1' style='z-index:10; position: absolute; top:0px; left:0px; background:red;' type='image/svg+xml' data='../accueil/module.svg'></object>

http://v1.xeolia.com/webkit_pb.png
Comment 17 jay 2009-10-19 03:42:42 PDT
#16

Matthieu that would be a separate bug, which may be dependent on this one.

You would need to check if the bug exists, and if not file a new one.

this bug relates to many types of object including java, svg etc....
it's not svg specific.
Comment 18 Matthieu Chaton 2009-10-19 03:54:36 PDT
#17
In fact I think it's more like the 17744 bug, which has been merged with this bug... I will reopen it, I've tested the object tag in safari with another type (not svg) and everything was fine
Comment 19 NSD 2010-01-03 20:05:41 PST
Whole lotta arguing about which bug this is or isn't a duplicate of, and not a whole lot of people doing anything but closing all of them repeatedly for 4 years straight. Which one is the officially recognized bug?
Comment 20 jay 2010-01-04 03:31:23 PST
Created attachment 45795 [details]
html background with 'default' white svg background demonstrated
Comment 21 jay 2010-01-04 03:33:21 PST
updated reduced testcase, as broken...

#15 Jonathan,
I have widened the scope to all, as tested by me today nightly on intel leopard fails 

#19 NSD,

I recognise your enthusiasm, however please bear in mind that its generally not helpful to add complaints as comments, they usually make it harder for coders to understand the issue. It is not uncommon for bugs to remain open for years.

this bug relates to the default background of the html <object> being transparent, not white.
other bugs that are not dupes, naturally differ in their descriptions.
Comment 22 jay 2010-01-04 03:34:47 PST
Comment on attachment 45795 [details]
html background with 'default' white svg background demonstrated 

linked content broken when webkit reorgandised
Comment 23 Cameron McCormack (:heycam) 2010-07-20 17:51:15 PDT
*** Bug 37416 has been marked as a duplicate of this bug. ***
Comment 24 Jeff Schiller 2010-09-20 08:00:51 PDT
Created attachment 68085 [details]
Reduced one-file test case using data: URL, no white should be visible
Comment 25 Jeff Schiller 2010-09-20 09:22:14 PDT
Created attachment 68095 [details]
Patch to make SVG documents embedded using <object>, <embed> have transparent backgrounds.

I'm not sure if it's correct for all object/embed frames to have a transparent background, so the attached patch only fixes this problem for SVG documents (HTML documents are already transparent).  Note that this does not fix Bug 27078, which fails because the MIME type is not served as SVG in that test.

Do we need a pixel test for this?  I'll need to re-learn how to do that...
Comment 26 Dirk Schulze 2010-09-20 09:31:13 PDT
Comment on attachment 68095 [details]
Patch to make SVG documents embedded using <object>, <embed> have transparent backgrounds.

Looks good, but needs a pixel test and a ChangeLog etry.
Comment 27 Jeff Schiller 2010-09-20 20:44:07 PDT
Created attachment 68181 [details]
This patch makes all backgrounds of SVG files served by reference in object, embed and iframes transparent.

This patch adds a test and fixes the case shown in Bug 27078 (a text/xml document with a SVG root).
Comment 28 Jeff Schiller 2010-09-21 07:26:43 PDT
Created attachment 68238 [details]
Test for text/xml documents with a <svg> root
Comment 29 Dirk Schulze 2010-09-21 07:36:15 PDT
Adding some Renderer guys to look at this patch as well. I'm not happy about the SVGNames include, but I guess we don't want to generalize the code to make background transparent for all document roots.
Comment 30 Dave Hyatt 2010-09-21 20:54:03 PDT
Comment on attachment 68181 [details]
This patch makes all backgrounds of SVG files served by reference in object, embed and iframes transparent.

SVGNames::svgTag includes the SVG namespace, so you don't need to do that extra comparison. Other than that looks fine to me.
Comment 31 Dave Hyatt 2010-09-21 20:58:29 PDT
Comment on attachment 68181 [details]
This patch makes all backgrounds of SVG files served by reference in object, embed and iframes transparent.

Actually I do have a good suggestion to make.  I think you could add a method to Document to answer the question of whether or not you are an SVG document or have a document element that is <svg>.  That would at least get the SVGNames stuff out of RenderBoxModelObject.  You could make the Document method return a sane value even when SVG is not enabled.  Then all the SVG-specific stuff could be in Document.
Comment 32 Jeff Schiller 2010-09-21 22:51:56 PDT
I liked your suggestion.  The first question to answer is:  If a document is served with MIME type text/xml, but it has a <svg> document root - is it a SVG Document?  

If so, then it really seems that DOMImplementation::createDocument() should be modified so that, if it's an XML document but it parses out with a <svg> root, we should create a SVGDocument (and destroy the Document?).  Any tips on the best way to do this without parsing the document twice?

The other option is to change isSVGDocument() to be similar to how isXHTMLDocument() is done, where a member bool can be set in the case of a Document parsing out with a <svg> root.  The thing I'm worried about is returning true for isSVGDocument() and not actually _being_ a SVGDocument.

The final option is to just add a isDocumentRootSVG() function to Document() and return true when it's a <svg>.  Is this what you're really suggesting?  Just seems odd to have both functions returning different values...
Comment 33 Jeff Schiller 2010-09-22 08:19:11 PDT
FYI, I checked out the Document object at https://bug-10687-attachments.webkit.org/attachment.cgi?id=68238 for various browsers.  Remember, this is a text/xml that happens to have a <svg> root:

WebKit: Document (not a SVGDocument)
Mozilla: Document (not a SVGDocument)
IE9: Document (not a SVGDocument)
Opera: SVGDocument
Comment 34 Jeff Schiller 2010-09-23 06:50:31 PDT
Created attachment 68511 [details]
Patch for making SVG documents and XML documents with SVG root transparent when embedded by reference.

This addresses krit's and dhyatt's concern of including SVGNames in RenderBoxModelObject.  This patch introduces Document::hasSVGRootNode() that can be used to determine if we are a SVG document or a XML document with a <svg> root.
Comment 35 Dave Hyatt 2010-09-23 10:28:46 PDT
Comment on attachment 68511 [details]
Patch for making SVG documents and XML documents with SVG root transparent when embedded by reference.

View in context: https://bugs.webkit.org/attachment.cgi?id=68511&action=review

> WebCore/dom/Document.cpp:2171
> +#endif

I don't think you need to cache this information.  Asking if the documentElement() has the SVG tag name every time is fine.  It's a quick check (it's AtomicString comparisons, which are very fast).

If you do still want to cache it, a better place would be in cacheDocumentElement().

> WebCore/dom/Document.h:388
> +    virtual bool hasSVGRootNode() const { return m_hasSVGRootNode; }

I don't think this needs to be virtual, since the same check that works for non-SVG documents (checking the root node) should also work for SVG documents.  It's faster to just do the root node tag check than it is to call a virtual function.

> WebCore/svg/SVGDocument.h:55
> +        virtual bool hasSVGRootNode() const { return true; }

Just remove this.
Comment 36 Jeff Schiller 2010-09-23 12:00:34 PDT
Created attachment 68556 [details]
Patch addressing dhyatt's comments (no caching of hasSVGRootNode).

Addressed dyhatt's comments.
Comment 37 WebKit Review Bot 2010-09-23 12:06:09 PDT
Attachment 68556 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/dom/Document.h:388:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/dom/Document.cpp:4030:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 38 Dave Hyatt 2010-09-23 12:11:31 PDT
Comment on attachment 68556 [details]
Patch addressing dhyatt's comments (no caching of hasSVGRootNode).

r=me
Comment 39 Jeff Schiller 2010-09-23 12:17:12 PDT
Created attachment 68561 [details]
Patch with webkit style fixups.
Comment 40 WebKit Commit Bot 2010-09-23 14:36:33 PDT
Comment on attachment 68561 [details]
Patch with webkit style fixups.

Clearing flags on attachment: 68561

Committed r68198: <http://trac.webkit.org/changeset/68198>
Comment 41 WebKit Commit Bot 2010-09-23 14:36:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 42 Alexey Proskuryakov 2011-01-27 11:31:04 PST
*** Bug 48462 has been marked as a duplicate of this bug. ***