| 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: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | OS X 10.5 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Renata Hodovan
2010-10-26 11:02:37 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)
(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. 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...
Created attachment 71988 [details]
Adding the missing super class call
(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. Comment on attachment 71988 [details]
Adding the missing super class call
Sorry, but this sentence need to go in. It's WebKit policy.
Created attachment 72039 [details]
Adding the missing super class call
Comment on attachment 72039 [details]
Adding the missing super class call
r=me
Comment on attachment 72039 [details] Adding the missing super class call Clearing flags on attachment: 72039 Committed r70654: <http://trac.webkit.org/changeset/70654> All reviewed patches have been landed. Closing bug. (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. > 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 :(
(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. > 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.
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. (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! :) http://trac.webkit.org/changeset/70667 might have broken GTK Linux 32-bit Debug |