Summary: | SVGLoad event fires too early | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dglazkov, morrita, podivilov, webkit.review.bot, zimmermann | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 77736 | ||||||
Bug Blocks: | 78002 | ||||||
Attachments: |
|
Description
Nikolas Zimmermann
2012-02-08 00:29:18 PST
Created attachment 126023 [details]
Patch
- It looks now it is possible to for the load event to be fired not only from <svg> but also from non-<svg> outermost elements. I'd love to have tests to cover such case. - The decision whether event should be fired or not is scattered to many places like finishParsingChildren() and part of dispatchSVGLoadEventToOutermostSVGElements(). How about to tweak the parameter of sendSVGLoadEventIfPossible into something like enum, to indicate the caller's context? Then we can make the decision inside that function. (In reply to comment #2) > - It looks now it is possible to for the load event to be fired not only from <svg> but also > from non-<svg> outermost elements. I'd love to have tests to cover such case. We have such tests cases, see eg. svg/custom/loadevents* The SVGLoad event is always fireable for any SVG element, not just outermost - that's a key difference between HTML/SVG load listeners. HTML onload always means listening to a window event, unlike for SVG, where its per element. > - The decision whether event should be fired or not is scattered to many places like > finishParsingChildren() and part of dispatchSVGLoadEventToOutermostSVGElements(). > How about to tweak the parameter of sendSVGLoadEventIfPossible into something like enum, > to indicate the caller's context? Then we can make the decision inside that function. Suggestion is not bad, but I'd like to keep it as simple as possible for now, to continue working on the SVG repaint test fixes - we can come back later and refactor this, once we consider eg. delayed SVGLoad events for <use> / <feImage> etc, which we don't support at the moment. Comment on attachment 126023 [details] Patch Attachment 126023 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11462325 New failing tests: svg/custom/repaint-on-image-bounds-change.svg Comment on attachment 126023 [details]
Patch
Okay. R+ed for now. Please skip chromium stuff before landing.
(In reply to comment #5) > (From update of attachment 126023 [details]) > Okay. R+ed for now. Please skip chromium stuff before landing. Thanks a lot, will do! Committed r107057: <http://trac.webkit.org/changeset/107057> repaint-on-image-bounds-change.svg still has wrong image in chromium: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fcustom%2Frepaint-on-image-bounds-change.svg&showExpectations=true (In reply to comment #8) > repaint-on-image-bounds-change.svg still has wrong image in chromium: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fcustom%2Frepaint-on-image-bounds-change.svg&showExpectations=true Oh the test in svg/custom, and svg/dynamic-updates haven't moved yet to the new repainting harness. They're all still flaky. A patch fixing this will come soon. (In reply to comment #9) > (In reply to comment #8) > > repaint-on-image-bounds-change.svg still has wrong image in chromium: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fcustom%2Frepaint-on-image-bounds-change.svg&showExpectations=true > Oh the test in svg/custom, and svg/dynamic-updates haven't moved yet to the new repainting harness. They're all still flaky. A patch fixing this will come soon. Bug 77736 fixed the tests in svg/custom - this problem should be gone now, can you confirm? (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > repaint-on-image-bounds-change.svg still has wrong image in chromium: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=svg%2Fcustom%2Frepaint-on-image-bounds-change.svg&showExpectations=true > > Oh the test in svg/custom, and svg/dynamic-updates haven't moved yet to the new repainting harness. They're all still flaky. A patch fixing this will come soon. > Bug 77736 fixed the tests in svg/custom - this problem should be gone now, can you confirm? Sorry, I meant to say 78115. (In reply to comment #10) > Bug 77736 fixed the tests in svg/custom - this problem should be gone now, can you confirm? I checked on my own, it passes now! Closing bug. |