Bug 48347

Summary:

Calling the super class of RenderSVGImage::updateFromElement is missing

Product:

WebKit

Reporter:

Renata Hodovan <rhodovan.u-szeged>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, dglazkov, eric, kling, krit, mdelaney7, webkit.review.bot, zherczeg, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Adding the missing super class call	zimmermann: review-
Adding the missing super class call	krit: review-, krit: commit-queue-
Adding the missing super class call	none

Renata Hodovan

Reported 2010-10-26 11:02:37 PDT

Calling the super class of RenderSVGImage::updateFromElement is missing

Attachments
Adding the missing super class call (1.13 KB, patch) 2010-10-26 11:14 PDT, Renata Hodovan	zimmermann: review-	Details Formatted Diff Diff
Adding the missing super class call (1.26 KB, patch) 2010-10-26 23:45 PDT, Renata Hodovan	krit: review- krit: commit-queue-	Details Formatted Diff Diff
Adding the missing super class call (1.41 KB, patch) 2010-10-27 08:33 PDT, Renata Hodovan	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Renata Hodovan

Comment 1 2010-10-26 11:14:51 PDT

Created attachment 71914 [details] Adding the missing super class call The patch fixes the failed dynamic-update tests of feConvolveMatrix as well (because they have SVGImage objects)

Andreas Kling

Comment 2 2010-10-26 11:16:58 PDT

(In reply to comment #1) > The patch fixes the failed dynamic-update tests of feConvolveMatrix as well (because they have SVGImage objects) This seems to imply that something should be unskipped and/or rebaselined.

Nikolas Zimmermann

Comment 3 2010-10-26 11:34:00 PDT

Comment on attachment 71914 [details] Adding the missing super class call Looks good, great catch! You need to write a better ChangeLog, saying what's fixed though...

Renata Hodovan

Comment 4 2010-10-26 23:45:59 PDT

Created attachment 71988 [details] Adding the missing super class call

Dirk Schulze

Comment 5 2010-10-27 01:10:04 PDT

(In reply to comment #4) > Created an attachment (id=71988) [details] > Adding the missing super class call I'd like to read a sentence in the ChangeLog that this is covered by dynamic updates of feConvolveMatrix, since every regression needs a regression test.

Dirk Schulze

Comment 6 2010-10-27 01:10:50 PDT

Comment on attachment 71988 [details] Adding the missing super class call Sorry, but this sentence need to go in. It's WebKit policy.

Renata Hodovan

Comment 7 2010-10-27 08:33:27 PDT

Created attachment 72039 [details] Adding the missing super class call

Dirk Schulze

Comment 8 2010-10-27 08:47:37 PDT

Comment on attachment 72039 [details] Adding the missing super class call r=me

WebKit Commit Bot

Comment 9 2010-10-27 09:10:21 PDT

Comment on attachment 72039 [details] Adding the missing super class call Clearing flags on attachment: 72039 Committed r70654: <http://trac.webkit.org/changeset/70654>

WebKit Commit Bot

Comment 10 2010-10-27 09:10:27 PDT

All reviewed patches have been landed. Closing bug.

Dimitri Glazkov (Google)

Comment 11 2010-10-27 10:06:21 PDT

(In reply to comment #10) > All reviewed patches have been landed. Closing bug. This patch affected a few pixel results, and they probably should be landed with the patch.

Zoltan Herczeg

Comment 12 2010-10-27 10:14:26 PDT

> This patch affected a few pixel results, and they probably should be landed with the patch. Chromium pixel tests are a difficult problem. Once I wanted to update some Chromium pixel tests, and asked Eric for help, but he said he use mac, and gave me some names. I wrote them, but they don't reply. The poblem is we couldn't compile a chromium port and run pixel tests, since we have no idea how it works :(

Dimitri Glazkov (Google)

Comment 13 2010-10-27 10:21:37 PDT

(In reply to comment #12) > > This patch affected a few pixel results, and they probably should be landed with the patch. > > Chromium pixel tests are a difficult problem. Once I wanted to update some Chromium pixel tests, and asked Eric for help, but he said he use mac, and gave me some names. I wrote them, but they don't reply. The poblem is we couldn't compile a chromium port and run pixel tests, since we have no idea how it works :( I'll take care of the Chromium pixel tests. However, I am positive this patch also affected platform/mac, for instance.

Zoltan Herczeg

Comment 14 2010-10-27 10:29:43 PDT

> I'll take care of the Chromium pixel tests. However, I am positive this patch also affected platform/mac, for instance. Exactly, in a good way. Those tests were working properly, until the patch mentioned in the ChangeLog made them wrong. So this patch just restores their oiginal state. Since the mac bots don't run pixel tests, this thing was hidden so far.

Zoltan Herczeg

Comment 15 2010-10-27 10:32:31 PDT

To be more precise: if you run mac pixel tests on the mainline, you get 10 fails. This patch restores 8 of them, and you get less fails.

Dimitri Glazkov (Google)

Comment 16 2010-10-27 10:45:11 PDT

(In reply to comment #15) > To be more precise: if you run mac pixel tests on the mainline, you get 10 fails. This patch restores 8 of them, and you get less fails. That's great! :)

WebKit Review Bot

Comment 17 2010-10-27 14:04:54 PDT

http://trac.webkit.org/changeset/70667 might have broken GTK Linux 32-bit Debug

Note You need to log in before you can comment on or make changes to this bug.