ScriptElementData is a protected membre variable of ScriptElement but it should be a private member variable since HTMLScriptElement and SVGScriptElement only use a small subset of ScriptElementData's methods.
Created attachment 73963 [details] cleanup
Attachment 73963 [details] did not build on win: Build output: http://queues.webkit.org/results/6102015
Created attachment 74005 [details] fixed haveFiredLoadEvent
Comment on attachment 74005 [details] fixed haveFiredLoadEvent View in context: https://bugs.webkit.org/attachment.cgi?id=74005&action=review > WebCore/dom/ScriptElement.h:95 > protected: > + bool haveFiredLoadEvent() const { return m_data.haveFiredLoadEvent(); } Why are any members of ScriptElementData protected? Is there a class that derives from ScriptElementData? If not, then protected and private mean the same thing, and these should be private rather than protected. > WebCore/html/HTMLScriptElement.h:41 > - bool haveFiredLoadEvent() const { return m_data.haveFiredLoadEvent(); } > + bool haveFiredLoadEvent() const { return ScriptElement::haveFiredLoadEvent(); } You can accomplish the same thing here by just removing this function altogether.
Comment on attachment 74005 [details] fixed haveFiredLoadEvent View in context: https://bugs.webkit.org/attachment.cgi?id=74005&action=review >> WebCore/dom/ScriptElement.h:95 >> + bool haveFiredLoadEvent() const { return m_data.haveFiredLoadEvent(); } > > Why are any members of ScriptElementData protected? Is there a class that derives from ScriptElementData? If not, then protected and private mean the same thing, and these should be private rather than protected. On this isn't a part of ScriptElementData. It's a part of ScriptElement, which is inherited by HTMLScriptElement and SVGScriptElement. >> WebCore/html/HTMLScriptElement.h:41 >> + bool haveFiredLoadEvent() const { return ScriptElement::haveFiredLoadEvent(); } > > You can accomplish the same thing here by just removing this function altogether. SVGScriptElement doesn't define this function so making haveFiredLoadEvent() public in ScriptElement means we're adding haveFiredLoadEvent to SVGScriptElement as well. If that's desirable, I'll do.
Committed r72172: <http://trac.webkit.org/changeset/72172>
(In reply to comment #5) > >> WebCore/html/HTMLScriptElement.h:41 > >> + bool haveFiredLoadEvent() const { return ScriptElement::haveFiredLoadEvent(); } > > > > You can accomplish the same thing here by just removing this function altogether. > > SVGScriptElement doesn't define this function so making haveFiredLoadEvent() public in ScriptElement means we're adding haveFiredLoadEvent to SVGScriptElement as well. If that's desirable, I'll do. Itβs fine to add it to SVGScriptElement. If we did not want it public in SVGScriptElement, there are two solutions: 1) Make this protected in ScriptElement, and put "using ScriptElement::haveFiredLoadingEvent" in the public section of HTMLScriptElement. 2) Make this public in ScriptElement, and put "using ScriptElement::haveFiredLoadingEvent" in the private section of SVGScriptElement.