Bug 99102

Summary:

SVGImage::data() returns NULL

Product:

WebKit

Reporter:

Jay Civelli <jcivelli>

Component:

SVG

Assignee:

Philip Rogers <pdr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

jcivelli, webkit.review.bot, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
First pass	eric: review+
Add back null check per Jay's request	webkit.review.bot: commit-queue-
Updating reviewer	none

Jay Civelli

Reported 2012-10-11 14:55:17 PDT

Calling SVGImage::data() returns NULL. It should return the actual SVG data.

Attachments
First pass (1.83 KB, patch) 2012-10-23 13:45 PDT, Philip Rogers	eric: review+	Details Formatted Diff Diff
Add back null check per Jay's request (1.84 KB, patch) 2012-10-23 17:10 PDT, Philip Rogers	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Updating reviewer (1.84 KB, patch) 2012-10-23 17:30 PDT, Philip Rogers	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Jay Civelli

Comment 1 2012-10-12 10:08:46 PDT

One way to repro is to run the Chromium webkit_unit_tests, more specifically WebPageNewSerializeTest,SVGImageDontCrash. webkit_unit_tests --gtest_filter=WebPageNewSerializeTest.SVGImageDontCrash In PageSerializer::addImageToResources() there is a LOG_ERROR that's printed when the SVG image content is missing.

Alexey Proskuryakov

Comment 2 2012-10-12 11:53:07 PDT

Is there any user observable effect?

Jay Civelli

Comment 3 2012-10-12 12:50:53 PDT

The only effect I am aware of is that MHTML generated for a page with SVG images will not include them (as it does not get the data).

Philip Rogers

Comment 4 2012-10-22 09:56:20 PDT

@Jay, were you able to create a repro for this (even if just in c++)?

Jay Civelli

Comment 5 2012-10-22 10:02:50 PDT

@Philip You can repro with the steps I described in comment #1. It's a unit-test that generate MHTML from a simple page with an SVG image.

Philip Rogers

Comment 6 2012-10-22 20:51:51 PDT

It looks like the image returned from imageForRenderer() does not contain the copied data() but image->image()->data() does. I should have a patch up tomorrow for this.

Philip Rogers

Comment 7 2012-10-23 13:45:52 PDT

Created attachment 170227 [details] First pass

Eric Seidel (no email)

Comment 8 2012-10-23 16:16:34 PDT

Comment on attachment 170227 [details] First pass LGTM. Do we know if there is a way to test this with LayoutTests instead of just a Chromium Unit Test?

Jay Civelli

Comment 9 2012-10-23 16:43:28 PDT

LGTM @eric We can't test that from a Layout test as page serialization cannot be triggered from a page.

Philip Rogers

Comment 10 2012-10-23 17:10:43 PDT

Created attachment 170271 [details] Add back null check per Jay's request

WebKit Review Bot

Comment 11 2012-10-23 17:27:55 PDT

Comment on attachment 170271 [details] Add back null check per Jay's request Rejecting attachment 170271 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/14529170

Philip Rogers

Comment 12 2012-10-23 17:30:00 PDT

Created attachment 170276 [details] Updating reviewer

WebKit Review Bot

Comment 13 2012-10-23 20:21:23 PDT

Comment on attachment 170276 [details] Updating reviewer Clearing flags on attachment: 170276 Committed r132295: <http://trac.webkit.org/changeset/132295>

WebKit Review Bot

Comment 14 2012-10-23 20:21:26 PDT

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.