Bug 72151 - Multiple foreign objects not rendered
Summary: Multiple foreign objects not rendered
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-11 10:41 PST by Florin Malita
Modified: 2011-11-14 20:22 PST (History)
6 users (show)

See Also:


Attachments
The second FO box is not visible. (546 bytes, text/html)
2011-11-11 10:41 PST, Florin Malita
no flags Details
Patch (5.52 KB, patch)
2011-11-11 10:59 PST, Florin Malita
no flags Details | Formatted Diff | Diff
Patch (5.39 KB, patch)
2011-11-11 15:48 PST, Florin Malita
no flags Details | Formatted Diff | Diff
Patch (10.08 KB, patch)
2011-11-14 08:31 PST, Florin Malita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florin Malita 2011-11-11 10:41:42 PST
Created attachment 114728 [details]
The second FO box is not visible.

When declaring multiple foreignObject elements, only the first one gets rendered.

I tracked this down to a parser problem where the SVG closing tags are not being case-adjusted (like the starting tags are). Thus, foreignObject closing tags are not detected and subsequent FO elements are inserted as children.

Patch coming soon.
Comment 1 Florin Malita 2011-11-11 10:59:29 PST
Created attachment 114732 [details]
Patch
Comment 2 Simon Fraser (smfr) 2011-11-11 15:13:27 PST
Comment on attachment 114732 [details]
Patch

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

> LayoutTests/svg/foreignObject/multiple-foreign-objects.html:7
> +    <body style="dmargin: 0;">

What is dmargin?

> LayoutTests/svg/foreignObject/multiple-foreign-objects.html:8
> +        <div style="background: #00ff00; width: 100px; height: 100px;">&nbsp;</div>

Would prefer "background-color: green;" No need for &nbsp;
Comment 3 Florin Malita 2011-11-11 15:48:44 PST
Created attachment 114790 [details]
Patch
Comment 4 Florin Malita 2011-11-11 16:03:58 PST
Comment on attachment 114732 [details]
Patch

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

Thanks, updated.

>> LayoutTests/svg/foreignObject/multiple-foreign-objects.html:7
>> +    <body style="dmargin: 0;">
> 
> What is dmargin?

My fat finger with some copy/paste help :-)

>> LayoutTests/svg/foreignObject/multiple-foreign-objects.html:8
>> +        <div style="background: #00ff00; width: 100px; height: 100px;">&nbsp;</div>
> 
> Would prefer "background-color: green;" No need for &nbsp;

Done.
Comment 5 WebKit Review Bot 2011-11-11 23:19:48 PST
Comment on attachment 114790 [details]
Patch

Attachment 114790 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10449711

New failing tests:
html5lib/runner.html
Comment 6 Florin Malita 2011-11-14 08:31:05 PST
Created attachment 114952 [details]
Patch
Comment 7 WebKit Review Bot 2011-11-14 09:31:46 PST
Comment on attachment 114952 [details]
Patch

Clearing flags on attachment: 114952

Committed r100157: <http://trac.webkit.org/changeset/100157>
Comment 8 WebKit Review Bot 2011-11-14 09:31:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2011-11-14 16:42:29 PST
This appears to have broken 5 tests:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r100157%20(34687)/results.html
Comment 10 Florin Malita 2011-11-14 20:22:35 PST
Thanks for catching this - you're referring to the 5 failing foreignObject tests, right? I actually think this only broke the last one (introduced in this patch, needs rebaselining) - the other 4 were introduced in r100045 and probably have been failing since (looks like they also need rebaselining on Leopard).

I'll get on it first thing tomorrow, but I'd also appreciate some advice on how to do a better job at catching these in the future.