Bug 203044

Summary: Load event must be fired only for the SVG structurally external elements and the outermost SVG element
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: SVGAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, clopez, commit-queue, dbates, dino, esprehn+autocc, ews-watchlist, fmalita, gyuyoung.kim, kangil.han, pdr, rniwa, schenney, sergio, simon.fraser, thorton, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 202013    
Attachments:
Description Flags
Image for the test case
none
test case
none
Patch
none
Archive of layout-test-results from ews211 for win-future
none
Patch none

Description Said Abou-Hallawa 2019-10-16 10:55:25 PDT
Open the attached test case.

Result: A green rectangle is displayed.
Expected: Two messages are displayed below the green rectangle saying two images were loaded. One is external and the other is data URI.
Comment 1 Said Abou-Hallawa 2019-10-16 10:56:12 PDT
Created attachment 381086 [details]
Image for the test case
Comment 2 Said Abou-Hallawa 2019-10-16 10:56:51 PDT
Created attachment 381087 [details]
test case
Comment 4 Said Abou-Hallawa 2019-10-17 15:25:10 PDT
Created attachment 381235 [details]
Patch
Comment 5 Simon Fraser (smfr) 2019-10-17 15:41:07 PDT
Comment on attachment 381235 [details]
Patch

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

> Source/WebCore/svg/SVGScriptElement.h:76
> +    Timer* loadEventTimer() final { return &m_loadEventTimer; }

Seems weird to expose the timer outside the class. Does anyone use it?
Comment 6 EWS Watchlist 2019-10-17 16:49:43 PDT
Comment on attachment 381235 [details]
Patch

Attachment 381235 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13144911

New failing tests:
fast/html/marquee-reparent-check.html
svg/custom/image-load-event.html
fast/dom/crash-moving-subtree-between-documents.html
fast/dom/focus-style-resolution.html
Comment 7 EWS Watchlist 2019-10-17 16:49:45 PDT
Created attachment 381249 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 8 Said Abou-Hallawa 2019-10-18 10:14:09 PDT
Created attachment 381312 [details]
Patch
Comment 9 WebKit Commit Bot 2019-10-18 11:29:19 PDT
Comment on attachment 381312 [details]
Patch

Clearing flags on attachment: 381312

Committed r251290: <https://trac.webkit.org/changeset/251290>
Comment 10 WebKit Commit Bot 2019-10-18 11:29:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2019-10-18 11:30:18 PDT
<rdar://problem/56413481>
Comment 12 Nikolas Zimmermann 2019-10-28 15:11:03 PDT
I am confused: svg/W3C-SVG-1.1/interact-events-01-b.svg should be affected by this patch, but I see no updates of the tests or the expectation.

The test exercises SVGLoad event dispatch to e.g. <rect> elements which previously finishParsingChildren() triggered, no?

mac/svg/W3C-SVG-1.1/interact-events-01-b-expected.png still shows the "old" expectation.

Said, can you clarify this?
Comment 13 Carlos Alberto Lopez Perez 2020-03-16 21:58:38 PDT
Comment on attachment 381312 [details]
Patch

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

> LayoutTests/imported/w3c/ChangeLog:16
> +        * web-platform-tests/svg/images/20x20.png: Added.
> +        Add an image which is referenced by these tests:
> +            web-platform-tests/svg/import/styling-pres-02-f-manual.svg
> +            web-platform-tests/svg/import/struct-use-01-t-manual.svg
> +            web-platform-tests/svg/import/interact-events-02-b-manual.svg

Have this fix been proposed back to WPT?
I don't see the file svg/images/20x20.png on the WPT repository.
Its there any open PR for it?