Summary: | Add beforeScript (or equivalent) event to fire on inline scripts and inline stylesheets | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||||||||
Component: | Page Loading | Assignee: | Brady Eidson <beidson> | ||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||
Severity: | Normal | CC: | abarth, ap, aroben, beidson, dglazkov, eric, ian, levin, mjs, ossy, webkit-ews, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | 41338, 41357 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Dave Hyatt
2010-06-11 11:28:13 PDT
I see why having an event for this would be useful, of course, but is beforeload the right thing for this? I'd hate to conflate the concept of "an external resource identified by a URL" and "some inline resource" without really thinking it through. I think Hyatt was thinking of adding something like Opera's BeforeScript event [1], except that it would apply to stylesheets, too. 1. http://www.opera.com/docs/userjs/specs/#evlistener (In reply to comment #3) > I think Hyatt was thinking of adding something like Opera's BeforeScript event [1], except that it would apply to stylesheets, too. > > 1. http://www.opera.com/docs/userjs/specs/#evlistener That's something I can totally get behind. Updating bug title to that effect. Still seems weird to have something named "beforeScript" fire on stylesheets, but, whatever. Yeah I think I'm going to call it beforeprocess. Created attachment 59805 [details]
IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.
Comment on attachment 59805 [details] IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch. > BeforeLoadEvent \ > + BeforeProcessEvent \ > Blob \ There's a tab here, but the other lines use spaces. > +const String& BeforeProcessEvent::text() const > +{ > + // FIXME - Return innerText for <style> elements and inline <script> elements, or the resource text for remote <script> elements > + static String* placeholder = new String; > + return *placeholder; > +} Can this really return const String& in all cases? Never needs to be a new string? > + return adoptRef(new BeforeProcessEvent()); No need for the parentheses here after the class name. > + void initBeforeProcessEvent(const AtomicString& type, bool canBubble, bool cancelable) > + { > + if (dispatched()) > + return; > + > + initEvent(type, canBubble, cancelable); > + } Why the dispatched() check? Doesn't initEvent already check that? > +private: > + BeforeProcessEvent() > + : Event(eventNames().beforeprocessEvent, false, true) > + {} Braces should be on separate lines. > + void initBeforeProcessEvent(in DOMString type, > + in boolean canBubble, > + in boolean cancelable); You should just put this all on one line. This indenting style isn't good, even if you copied it from another IDL file. Otherwise looks OK. Seems fine to land. Scaffolding landed in r61964 Actually hookup coming soon. This should be easy to wire into the HTML5ScriptRunner. You should make sure to test what document.write() looks like during onBeforeProcess (where should the insertion point be?). There's alot more than just the HTMLScriptRunner to patch into, but I've found many places. Patch will be up monday morning. Comment on attachment 59805 [details]
IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.
Cool idea. Should we propose these event for HTML.next since we're adding them to the platform? We need to be careful with the interaction of setText and external scripts.
I wonder if we can generalize this approach to give folks an XSS defense... I'll have to think about that more.
WebCore/DerivedSources.make:56
+ BeforeProcessEvent \
Bad indent
WebCore/dom/BeforeProcessEvent.cpp:34
+ // FIXME - Return innerText for <style> elements and inline <script> elements, or the resource text for remote <script> elements
Presumably limited to inline style as well?
WebCore/dom/BeforeProcessEvent.cpp:42
+ // remote <script> elements, replacing it with an innerText property.
Woah, that sounds scary. I'd like to look at this in the next patch.
(In reply to comment #11) > (From update of attachment 59805 [details]) > Cool idea. Should we propose these event for HTML.next since we're adding them to the platform? We need to be careful with the interaction of setText and external scripts. ... > WebCore/dom/BeforeProcessEvent.cpp:42 > + // remote <script> elements, replacing it with an innerText property. > Woah, that sounds scary. I'd like to look at this in the next patch. Yah, as I've gotten much closer to finishing the patch and have been filling out layouttests, I realize that may not be realistic. We'll see in a bit. Created attachment 59958 [details]
Rework createAttributeEventListener
Rework createAttributeEventListener.
For the next patch, it's important that JSLazyEventListener's know what their original Node* was, event for window events created from <svg>, <frameset>, and <body> elements.
Attachment 59958 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3357015 Comment on attachment 59958 [details] Rework createAttributeEventListener > +PassRefPtr<JSLazyEventListener> createWindowAttributeEventListener(Node* windowEquivalentNode, Attribute* attr) > { > + Frame* frame = windowEquivalentNode->document()->frame(); I would reinforce this by asserting that the node is actually one of those elements. Oh, and maybe pass Element*, not Node*. > - return JSLazyEventListener::create(attr->localName().string(), eventParameterName(frame->document()->isSVGDocument()), attr->value(), 0, sourceURL, lineNumber, wrapper, mainThreadNormalWorld()); > + return JSLazyEventListener::create(attr->localName().string(), eventParameterName(frame->document()->isSVGDocument()), attr->value(), windowEquivalentNode, sourceURL, lineNumber, wrapper, mainThreadNormalWorld()); Please add a comment in JSLazyEventListener.h explaining that m_wrapper isn't necessarily a wrapper for m_originalNode, but is created from it if the wrapper is null. You may want to remove "if (m_originalNode)" condition in JSLazyEventListener::initializeJSFunction(), as it's seemingly always true now. It will be a fairly confusing detail that wrapper doesn't always match m_originalNode. r=me with comments addressed. (In reply to comment #14) > Attachment 59958 [details] did not build on chromium: > Build output: http://webkit-commit-queue.appspot.com/results/3357015 I'm ready to land, but this chromium build failure makes me nervous. I don't understand how the failures listed in the log could possibly have occurred as a result of this patch, if their build *was* already working. Especially with all the other builds continuing to work. Ahhh. V8 vs JSC. Great. > I'm ready to land, but this chromium build failure makes me nervous. > > I don't understand how the failures listed in the log could possibly have occurred as a result of this patch, if their build *was* already working. In general, the EWS checks that it can build before applying the patch, so a broken build just causes the bots to wait. > Ahhh. V8 vs JSC. Great. I believe the current social contract is that you can land patches that break because of V8 and it's the Chromium project's responsibility to clean up. Today, the folks on cleanup duty are dglazkov and mnaganov. They might appreciate if you gave them a heads up first. Created attachment 59963 [details]
Putting up for review just to see if Chromium's build is fixed.
Okay, thanks for the heads up. I'll land my patch that attempts to keep V8 going, but it might not quite work. (I note that at least dglazkov is CC'ed on this bug) Landed in r62052 http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal (In reply to comment #24) > http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal Why doesn't this type of message have a link to a build log of the suspected build breakage? (In reply to comment #24) > http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal It passed EWS and it didn't break Qt Release Linux. How is Qt Linux Release minimal different? > > http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal > > Why doesn't this type of message have a link to a build log of the suspected build breakage? That's a good suggestion. In the case of broken tests, we're planning to have it list the broken tests. Improving these tools is on the agenda, but we've been stepping back a bit to understand which are the important problems. > > http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal > > It passed EWS and it didn't break Qt Release Linux. How is Qt Linux Release minimal different? We should ask a Qt expert, but I think minimal tries to compile with all the feature defines turned off. I'm not sure exactly why this bot is useful, but it tends to build very quickly. (In reply to comment #19) > > Ahhh. V8 vs JSC. Great. > > I believe the current social contract is that you can land patches that break because of V8 and it's the Chromium project's responsibility to clean up. Today, the folks on cleanup duty are dglazkov and mnaganov. They might appreciate if you gave them a heads up first. Actually levin and mnaganov :) Thanks for attempting to fix it Brady! It made the final tweak completely trivial: http://trac.webkit.org/changeset/62073 After discussions with Maciej today hashing out an awful recursion issue with the attribute listeners, we decided on a few of things: 1 - We shouldn't support the onbeforeprocess attribute for scripts. The only way to add listeners for the event should be in script itself, using addEventListener. 2 - We can still support on onbeforeprocess attribute for <style> since there's no recursion risk or other problems caused by it. 3 - People interested in listening to the event for scripts might not be interested in listening to it for style, and vice versa. The conclusion of all of this? Splitting this into beforeprocessscript and beforeprocessstyle Retitling to the effect that this bug is about script-only, and https://bugs.webkit.org/show_bug.cgi?id=41328 covers style. I have the script stuff all working, but will first attach a patch that does the "beforeProcess -> beforeProcessScript and beforeProcessStyle" divide. Created attachment 59984 [details] Split BeforeProcess into BeforeProcessScript and BeforeProcessStyle Split BeforeProcess into BeforeProcessScript and BeforeProcessStyle After this point, style work will take place in https://bugs.webkit.org/show_bug.cgi?id=41328 Is there any spec-work being done in this area? We commonly r- patches which have no spec-basis (see the haptic CSS stuff as of late). Attachment 59984 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3356035 Comment on attachment 59984 [details] Split BeforeProcess into BeforeProcessScript and BeforeProcessStyle > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 62085) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,67 @@ > +2010-06-28 Brady Eidson <beidson@apple.com> > + > + Reviewed by NOBODY (OOPS!). > + > + More progress on https://bugs.webkit.org/show_bug.cgi?id=40484 > + > + It has become apparent that beforeprocessscript and beforeprocessstyle need to be separate, > + and also that onbeforeprocessscript should not exist, while onbeforeprocessstyle should. > + Please add more information here. > + void initBeforeProcessScriptEvent(const AtomicString& type, bool canBubble, bool cancelable) > + { > + initEvent(type, canBubble, cancelable); > + } Should this take a text argument as well? (Same question for the style event as well. Do you plan on adding BeforeProcessEventConstructor and BeforeProcessStyleEventConstructor to DOMWindow.idl in a follow up patch? Seems fine as a stepping stone. r=me. (In reply to comment #33) > Is there any spec-work being done in this area? We commonly r- patches which have no spec-basis (see the haptic CSS stuff as of late). Definitely plan to submit to standards bodies after we stop churning on our own ideas. As I discuss this with folks on IRC, it appears we'll end up reverting to Operas beforeScript and adding beforeStyle, so there'd already be multiple implementors for part of it. Stay tuned on that. (In reply to comment #35) > (From update of attachment 59984 [details]) > > Seems fine as a stepping stone. r=me. Thanks for the review. I'm going to hold off on landing this, as Dan pointed out the following: 1 - Opera already has beforeScript 2 - Our original motivation for calling our event beforeProcess was that we intended to overload it for both scripts and styles 3 - In implementing things, it became clear that we should have two different events for script and style 4 - So why not fall back to Opera's beforeScript for our script event, and add our own beforeStyle event? Even though I have the script half of this implemented, with layouttests, and ready to go, I need to back off my mad rush to try to finish this work before my vacation and get some consensus on that idea before landing this stepping stone. > Even though I have the script half of this implemented, with layouttests, and ready to go, I need to back off my mad rush to try to finish this work before my vacation and get some consensus on that idea before landing this stepping stone.
Would be willing to post your patch somewhere? I'd be happy to work on it while you're away.
By the way, Qt still fails to compile because of http://trac.webkit.org/changeset/62052 g++ -c -m32 -pipe -Wall -Wextra -Wreturn-type -fno-strict-aliasing -Wcast-align -Wchar-subscripts -Wformat-security -Wreturn-type -Wno-unused-parameter -Wno-sign-compare -Wno-switch -Wno-switch-enum -Wundef -Wmissing-noreturn -Winit-self -ffunction-sections -fdata-sections -O2 -fvisibility=hidden -fvisibility-inlines-hidden -D_REENTRANT -fPIC -DENABLE_3D_RENDERING=0 -DENABLE_CHANNEL_MESSAGING=0 -DENABLE_DATABASE=0 -DENABLE_DATALIST=0 -DENABLE_DOM_STORAGE=0 -DENABLE_EVENTSOURCE=0 -DENABLE_FILTERS=0 -DENABLE_ICONDATABASE=0 -DENABLE_JAVASCRIPT_DEBUGGER=0 -DENABLE_NOTIFICATIONS=0 -DENABLE_OFFLINE_WEB_APPLICATIONS=0 -DENABLE_RUBY=0 -DENABLE_SANDBOX=0 -DENABLE_SHARED_WORKERS=0 -DENABLE_SVG=0 -DENABLE_SVG_ANIMATION=0 -DENABLE_SVG_AS_IMAGE=0 -DENABLE_SVG_FONTS=0 -DENABLE_SVG_FOREIGN_OBJECT=0 -DENABLE_SVG_USE=0 -DENABLE_TILED_BACKING_STORE=0 -DENABLE_VIDEO=0 -DENABLE_WEB_SOCKETS=0 -DENABLE_WORKERS=0 -DENABLE_XPATH=0 -DENABLE_XSLT=0 -DBUILDING_QT__=1 -DWTF_USE_ACCELERATED_COMPOSITING -DNDEBUG -DQT_MAKEDLL -DBUILD_WEBKIT -DBUILDING_QT__ -DBUILDING_JavaScriptCore -DBUILDING_WTF -DENABLE_NETSCAPE_PLUGIN_API=0 -DENABLE_ORIENTATION_EVENTS=0 -DENABLE_DIRECTORY_UPLOAD=0 -DENABLE_SQLITE=0 -DENABLE_DASHBOARD_SUPPORT=0 -DENABLE_WCSS=0 -DENABLE_WML=0 -DENABLE_XHTMLMP=0 -DENABLE_DATAGRID=0 -DENABLE_METER_TAG=1 -DENABLE_PROGRESS_TAG=1 -DENABLE_BLOB_SLICE=0 -DENABLE_IMAGE_RESIZER=0 -DENABLE_INPUT_SPEECH=0 -DENABLE_SVG_FONTS=0 -DENABLE_SVG_FOREIGN_OBJECT=0 -DENABLE_SVG_ANIMATION=0 -DENABLE_SVG_AS_IMAGE=0 -DENABLE_SVG_USE=0 -DENABLE_WEB_TIMING=0 -DENABLE_TOUCH_EVENTS=1 -DQT_NO_DEBUG -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -DQT_SHARED -I/usr/local/Trolltech/Qt-4.6.2/mkspecs/linux-g++-32 -I../../../WebCore -I/usr/local/Trolltech/Qt-4.6.2/include/QtCore -I/usr/local/Trolltech/Qt-4.6.2/include/QtNetwork -I/usr/local/Trolltech/Qt-4.6.2/include/QtGui -I/usr/local/Trolltech/Qt-4.6.2/include -I../../../WebCore/bridge/qt -I../../../WebCore/page/qt -I../../../WebCore/platform/graphics/qt -I../../../WebCore/platform/network/qt -I../../../WebCore/platform/qt -I../../../WebKit/qt/Api -I../../../WebKit/qt/WebCoreSupport -I../../../WebCore -I../../../WebCore/accessibility -I../../../WebCore/bindings/js -I../../../WebCore/bridge -I../../../WebCore/bridge/c -I../../../WebCore/bridge/jsc -I../../../WebCore/css -I../../../WebCore/dom -I../../../WebCore/dom/default -I../../../WebCore/editing -I../../../WebCore/history -I../../../WebCore/html -I../../../WebCore/html/canvas -I../../../WebCore/inspector -I../../../WebCore/loader -I../../../WebCore/loader/appcache -I../../../WebCore/loader/archive -I../../../WebCore/loader/icon -I../../../WebCore/mathml -I../../../WebCore/notifications -I../../../WebCore/page -I../../../WebCore/page/animation -I../../../WebCore/platform -I../../../WebCore/platform/animation -I../../../WebCore/platform/graphics -I../../../WebCore/platform/graphics/filters -I../../../WebCore/platform/graphics/transforms -I../../../WebCore/platform/image-decoders -I../../../WebCore/platform/mock -I../../../WebCore/platform/network -I../../../WebCore/platform/sql -I../../../WebCore/platform/text -I../../../WebCore/platform/text/transcoder -I../../../WebCore/plugins -I../../../WebCore/rendering -I../../../WebCore/rendering/style -I../../../WebCore/storage -I../../../WebCore/svg -I../../../WebCore/svg/animation -I../../../WebCore/svg/graphics -I../../../WebCore/svg/graphics/filters -I../../../WebCore/websockets -I../../../WebCore/wml -I../../../WebCore/workers -I../../../WebCore/xml -Igenerated -I../../../JavaScriptCore -I../../../../build -I../../../JavaScriptCore/assembler -I../../../JavaScriptCore/bytecode -I../../../JavaScriptCore/bytecompiler -I../../../JavaScriptCore/debugger -I../../../JavaScriptCore/interpreter -I../../../JavaScriptCore/jit -I../../../JavaScriptCore/parser -I../../../JavaScriptCore/pcre -I../../../JavaScriptCore/profiler -I../../../JavaScriptCore/runtime -I../../../JavaScriptCore/wtf -I../../../JavaScriptCore/wtf/symbian -I../../../JavaScriptCore/wtf/unicode -I../../../JavaScriptCore/yarr -I../../../JavaScriptCore/API -I../../../JavaScriptCore/ForwardingHeaders -I../JavaScriptCore/generated -I../include/QtWebKit -I. -I../../../WebCore -I. -o obj/release/ScriptEventListener.o ../../../WebCore/bindings/js/ScriptEventListener.cpp ../../../WebCore/bindings/js/ScriptEventListener.cpp:40:22: error: SVGNames.h: No such file or directory g++ -c -m32 -pipe -Wall -Wextra -Wreturn-type -fno-strict-aliasing -Wcast-align -Wchar-subscripts -Wformat-security -Wreturn-type -Wno-unused-parameter -Wno-sign-compare -Wno-switch -Wno-switch-enum -Wundef -Wmissing-noreturn -Winit-self -ffunction-sections -fdata-sections -O2 -fvisibility=hidden -fvisibility-inlines-hidden -D_REENTRANT -fPIC -DENABLE_3D_RENDERING=0 -DENABLE_CHANNEL_MESSAGING=0 -DENABLE_DATABASE=0 -DENABLE_DATALIST=0 -DENABLE_DOM_STORAGE=0 -DENABLE_EVENTSOURCE=0 -DENABLE_FILTERS=0 -DENABLE_ICONDATABASE=0 -DENABLE_JAVASCRIPT_DEBUGGER=0 -DENABLE_NOTIFICATIONS=0 -DENABLE_OFFLINE_WEB_APPLICATIONS=0 -DENABLE_RUBY=0 -DENABLE_SANDBOX=0 -DENABLE_SHARED_WORKERS=0 -DENABLE_SVG=0 -DENABLE_SVG_ANIMATION=0 -DENABLE_SVG_AS_IMAGE=0 -DENABLE_SVG_FONTS=0 -DENABLE_SVG_FOREIGN_OBJECT=0 -DENABLE_SVG_USE=0 -DENABLE_TILED_BACKING_STORE=0 -DENABLE_VIDEO=0 -DENABLE_WEB_SOCKETS=0 -DENABLE_WORKERS=0 -DENABLE_XPATH=0 -DENABLE_XSLT=0 -DBUILDING_QT__=1 -DWTF_USE_ACCELERATED_COMPOSITING -DNDEBUG -DQT_MAKEDLL -DBUILD_WEBKIT -DBUILDING_QT__ -DBUILDING_JavaScriptCore -DBUILDING_WTF -DENABLE_NETSCAPE_PLUGIN_API=0 -DENABLE_ORIENTATION_EVENTS=0 -DENABLE_DIRECTORY_UPLOAD=0 -DENABLE_SQLITE=0 -DENABLE_DASHBOARD_SUPPORT=0 -DENABLE_WCSS=0 -DENABLE_WML=0 -DENABLE_XHTMLMP=0 -DENABLE_DATAGRID=0 -DENABLE_METER_TAG=1 -DENABLE_PROGRESS_TAG=1 -DENABLE_BLOB_SLICE=0 -DENABLE_IMAGE_RESIZER=0 -DENABLE_INPUT_SPEECH=0 -DENABLE_SVG_FONTS=0 -DENABLE_SVG_FOREIGN_OBJECT=0 -DENABLE_SVG_ANIMATION=0 -DENABLE_SVG_AS_IMAGE=0 -DENABLE_SVG_USE=0 -DENABLE_WEB_TIMING=0 -DENABLE_TOUCH_EVENTS=1 -DQT_NO_DEBUG -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -DQT_SHARED -I/usr/local/Trolltech/Qt-4.6.2/mkspecs/linux-g++-32 -I../../../WebCore -I/usr/local/Trolltech/Qt-4.6.2/include/QtCore -I/usr/local/Trolltech/Qt-4.6.2/include/QtNetwork -I/usr/local/Trolltech/Qt-4.6.2/include/QtGui -I/usr/local/Trolltech/Qt-4.6.2/include -I../../../WebCore/bridge/qt -I../../../WebCore/page/qt -I../../../WebCore/platform/graphics/qt -I../../../WebCore/platform/network/qt -I../../../WebCore/platform/qt -I../../../WebKit/qt/Api -I../../../WebKit/qt/WebCoreSupport -I../../../WebCore -I../../../WebCore/accessibility -I../../../WebCore/bindings/js -I../../../WebCore/bridge -I../../../WebCore/bridge/c -I../../../WebCore/bridge/jsc -I../../../WebCore/css -I../../../WebCore/dom -I../../../WebCore/dom/default -I../../../WebCore/editing -I../../../WebCore/history -I../../../WebCore/html -I../../../WebCore/html/canvas -I../../../WebCore/inspector -I../../../WebCore/loader -I../../../WebCore/loader/appcache -I../../../WebCore/loader/archive -I../../../WebCore/loader/icon -I../../../WebCore/mathml -I../../../WebCore/notifications -I../../../WebCore/page -I../../../WebCore/page/animation -I../../../WebCore/platform -I../../../WebCore/platform/animation -I../../../WebCore/platform/graphics -I../../../WebCore/platform/graphics/filters -I../../../WebCore/platform/graphics/transforms -I../../../WebCore/platform/image-decoders -I../../../WebCore/platform/mock -I../../../WebCore/platform/network -I../../../WebCore/platform/sql -I../../../WebCore/platform/text -I../../../WebCore/platform/text/transcoder -I../../../WebCore/plugins -I../../../WebCore/rendering -I../../../WebCore/rendering/style -I../../../WebCore/storage -I../../../WebCore/svg -I../../../WebCore/svg/animation -I../../../WebCore/svg/graphics -I../../../WebCore/svg/graphics/filters -I../../../WebCore/websockets -I../../../WebCore/wml -I../../../WebCore/workers -I../../../WebCore/xml -Igenerated -I../../../JavaScriptCore -I../../../../build -I../../../JavaScriptCore/assembler -I../../../JavaScriptCore/bytecode -I../../../JavaScriptCore/bytecompiler -I../../../JavaScriptCore/debugger -I../../../JavaScriptCore/interpreter -I../../../JavaScriptCore/jit -I../../../JavaScriptCore/parser -I../../../JavaScriptCore/pcre -I../../../JavaScriptCore/profiler -I../../../JavaScriptCore/runtime -I../../../JavaScriptCore/wtf -I../../../JavaScriptCore/wtf/symbian -I../../../JavaScriptCore/wtf/unicode -I../../../JavaScriptCore/yarr -I../../../JavaScriptCore/API -I../../../JavaScriptCore/ForwardingHeaders -I../JavaScriptCore/generated -I../include/QtWebKit -I. -I../../../WebCore -I. -o obj/release/ScriptFunctionCall.o ../../../WebCore/bindings/js/ScriptFunctionCall.cpp ../../../WebCore/bindings/js/ScriptEventListener.cpp: In function ‘WTF::PassRefPtr<WebCore::JSLazyEventListener> WebCore::createWindowAttributeEventListener(WebCore::Element*, WebCore::Attribute*)’: ../../../WebCore/bindings/js/ScriptEventListener.cpp:87: error: invalid use of incomplete type ‘struct WebCore::Element’ ../../../WebCore/dom/Node.h:52: error: forward declaration of ‘struct WebCore::Element’ ../../../WebCore/bindings/js/ScriptEventListener.cpp:110: error: no matching function for call to ‘WebCore::JSLazyEventListener::create(const WebCore::String&, const WebCore::String&, const WebCore::AtomicString&, WebCore::Element*&, WebCore::String&, int&, JSC::JSObject*&, WebCore::DOMWrapperWorld*)’ ../../../WebCore/bindings/js/JSLazyEventListener.h:32: note: candidates are: static WTF::PassRefPtr<WebCore::JSLazyEventListener> WebCore::JSLazyEventListener::create(const WebCore::String&, const WebCore::String&, const WebCore::String&, WebCore::Node*, const WebCore::String&, int, JSC::JSObject*, WebCore::DOMWrapperWorld*) http://build.webkit.org/builders/Qt%20Linux%20Release%20minimal/builds/3925/steps/compile-webkit/logs/stdio Comment on attachment 59805 [details] IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch. Cleared Darin Adler's review+ from obsolete attachment 59805 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 59958 [details] Rework createAttributeEventListener Cleared Alexey Proskuryakov's review+ from obsolete attachment 59958 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 59963 [details] Putting up for review just to see if Chromium's build is fixed. This is causing crashes in the regression tests. See bug 41352. Unsure if this bug still needs to be open. It appears this was never landed. The reasons why we wanted this have passed. I think Sam is nuking the code. |