Bug 40484 – Add beforeScript (or equivalent) event to fire on inline scripts and inline stylesheets

• Component: Page Loading
• Status: RESOLVED WONTFIX

[Dave Hyatt]

beforeload should fire on inline scripts and stylesheets to possibly halt their processing.

[Brian Weinstein]

<rdar://problem/8084335>

[Brady Eidson]

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.

[Adam Roben (:aroben)]

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

[Brady Eidson]

(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.

[Dave Hyatt]

Yeah I think I'm going to call it beforeprocess.

[Brady Eidson]

Created attachment 59805 [details]
IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.

[Darin Adler]

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.

[Brady Eidson]

Scaffolding landed in r61964

Actually hookup coming soon.

[Eric Seidel (no email)]

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?).

[Brady Eidson]

There's alot more than just the HTMLScriptRunner to patch into, but I've found many places.  Patch will be up monday morning.

[Adam Barth]

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.

[Brady Eidson]

(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.

[Brady Eidson]

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.

[WebKit Review Bot]

Attachment 59958 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3357015

[Alexey Proskuryakov]

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.

[Brady Eidson]

(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.

[Brady Eidson]

Especially with all the other builds continuing to work.

[Brady Eidson]

Ahhh.  V8 vs JSC.  Great.

[Adam Barth]

> 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.

[Brady Eidson]

Created attachment 59963 [details]
Putting up for review just to see if Chromium's build is fixed.

[Brady Eidson]

Okay, thanks for the heads up.

I'll land my patch that attempts to keep V8 going, but it might not quite work.

[Brady Eidson]

(I note that at least dglazkov is CC'ed on this bug)

[Brady Eidson]

Landed in r62052

[WebKit Review Bot]

http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal

[Brady Eidson]

(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?

[Brady Eidson]

(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?

[Adam Barth]

> > 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.

[David Levin]

(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

[Brady Eidson]

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

[Brady Eidson]

Retitling to the effect that this bug is about script-only, and https://bugs.webkit.org/show_bug.cgi?id=41328 covers style.

[Brady Eidson]

I have the script stuff all working, but will first attach a patch that does the "beforeProcess -> beforeProcessScript and beforeProcessStyle" divide.

[Brady Eidson]

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

[Eric Seidel (no email)]

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).

[Early Warning System Bot]

Attachment 59984 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3356035

[Sam Weinig]

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 @@
> [phone]  Brady Eidson  <[e-mail]>
> +
> +        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.

[Brady Eidson]

(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.

[Brady Eidson]

(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.

[Adam Barth]

> 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.

[Adam Barth]

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

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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.

[Adam Roben (:aroben)]

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.

[Eric Seidel (no email)]

Unsure if this bug still needs to be open.

[Eric Seidel (no email)]

It appears this was never landed.

[Brady Eidson]

The reasons why we wanted this have passed.  I think Sam is nuking the code.