Drop SVGDocument as per the SVG2 specification: - https://www.w3.org/TR/SVG2/struct.html#InterfaceDocumentExtensions SVGDocument has been merged into Document. Chrome and Edge have dropped SVGDocument already, Firefox has not.
Created attachment 284913 [details] WIP Patch
Comment on attachment 284913 [details] WIP Patch Attachment 284913 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1776071 New failing tests: svg/custom/manually-parsed-svg-allowed-in-dashboard.html
Created attachment 284921 [details] Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 284913 [details] WIP Patch Attachment 284913 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1776080 New failing tests: svg/custom/manually-parsed-svg-allowed-in-dashboard.html
Created attachment 284922 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 285531 [details] Patch
Comment on attachment 285531 [details] Patch Attachment 285531 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1830854 New failing tests: svg/custom/frame-getSVGDocument.html
Created attachment 285532 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 285531 [details] Patch Attachment 285531 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1830866 New failing tests: svg/custom/frame-getSVGDocument.html
Created attachment 285533 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 285531 [details] Patch Attachment 285531 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1830868 New failing tests: svg/custom/frame-getSVGDocument.html
Created attachment 285534 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.5
Comment on attachment 285531 [details] Patch Attachment 285531 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1830871 New failing tests: svg/custom/frame-getSVGDocument.html
Created attachment 285535 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 285539 [details] Patch
Created attachment 285540 [details] Patch
Comment on attachment 285540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285540&action=review Seems OK if it compiles. > Source/WebCore/dom/Document.h:426 > + SVGSVGElement* svgRootElement() const; Does this have to be a member of Document? How about making it a static member function in the SVGSVGElement class that takes a Document& instead? Or is this needed for the bindings? > Source/WebCore/svg/SVGDocument.h:35 > + static SVGSVGElement* rootElement(Document&); Why are we defining this? Who is calling it? > Source/WebCore/svg/SVGDocument.idl:26 > +partial interface Document { > +#if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT > readonly attribute SVGSVGElement? rootElement; > +#endif > }; What code does this compile down to? I don’t understand how it works.
Comment on attachment 285540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285540&action=review >> Source/WebCore/dom/Document.h:426 >> + SVGSVGElement* svgRootElement() const; > > Does this have to be a member of Document? How about making it a static member function in the SVGSVGElement class that takes a Document& instead? Or is this needed for the bindings? No, we don't have to add this getter here. The only requirement for the bindings is that we have the static rootElement(Document&) function on SVGDocument. I can drop this getter and only use the static function on SVGDocument. I just thought this looked nicer at call sites. >> Source/WebCore/svg/SVGDocument.h:35 >> + static SVGSVGElement* rootElement(Document&); > > Why are we defining this? Who is calling it? This one is for the bindings.
Comment on attachment 285540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285540&action=review >>> Source/WebCore/dom/Document.h:426 >>> + SVGSVGElement* svgRootElement() const; >> >> Does this have to be a member of Document? How about making it a static member function in the SVGSVGElement class that takes a Document& instead? Or is this needed for the bindings? > > No, we don't have to add this getter here. The only requirement for the bindings is that we have the static rootElement(Document&) function on SVGDocument. I can drop this getter and only use the static function on SVGDocument. I just thought this looked nicer at call sites. How about a static function on SVGSVGElement instead? Do we need an SVGDocument class at all long term? >>> Source/WebCore/svg/SVGDocument.h:35 >>> + static SVGSVGElement* rootElement(Document&); >> >> Why are we defining this? Who is calling it? > > This one is for the bindings. How do the bindings know what function to call? I don’t see the string SVGDocument anywhere in the IDL file.
Created attachment 285543 [details] Patch
Comment on attachment 285540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285540&action=review >>>> Source/WebCore/svg/SVGDocument.h:35 >>>> + static SVGSVGElement* rootElement(Document&); >>> >>> Why are we defining this? Who is calling it? >> >> This one is for the bindings. > > How do the bindings know what function to call? I don’t see the string SVGDocument anywhere in the IDL file. I believe it uses the IDL filename (in this case SVGElement.idl).
Comment on attachment 285540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285540&action=review >>>> Source/WebCore/dom/Document.h:426 >>>> + SVGSVGElement* svgRootElement() const; >>> >>> Does this have to be a member of Document? How about making it a static member function in the SVGSVGElement class that takes a Document& instead? Or is this needed for the bindings? >> >> No, we don't have to add this getter here. The only requirement for the bindings is that we have the static rootElement(Document&) function on SVGDocument. I can drop this getter and only use the static function on SVGDocument. I just thought this looked nicer at call sites. > > How about a static function on SVGSVGElement instead? Do we need an SVGDocument class at all long term? This would require adding support some new IDL extended attribute and tweaking the bindings generator to specify where the static function is defined. Currently, the bindings generator relies on the IDL filename (in this case SVGDocument). I don't know if it is worth the trouble. Is my latest patch acceptable?
(In reply to comment #22) > This would require adding support some new IDL extended attribute and > tweaking the bindings generator to specify where the static function is > defined. Currently, the bindings generator relies on the IDL filename (in > this case SVGDocument). > I don't know if it is worth the trouble. Is my latest patch acceptable? Sure this is fine. I didn't realize that the bindings generator used the name of the file for partial interfaces in this fashion. It's completely reasonable to go with that. If we were planning to remove the class SVGDocument, then SVGSVGElement would be a good home for this single function.
Created attachment 285545 [details] Patch
Created attachment 285546 [details] Patch
Comment on attachment 285546 [details] Patch Clearing flags on attachment: 285546 Committed r204246: <http://trac.webkit.org/changeset/204246>
All reviewed patches have been landed. Closing bug.