Bug 40484

Summary: Add beforeScript (or equivalent) event to fire on inline scripts and inline stylesheets
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Page LoadingAssignee: 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 Flags
IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.
none
Rework createAttributeEventListener
none
Putting up for review just to see if Chromium's build is fixed.
beidson: review-, beidson: commit-queue-
Split BeforeProcess into BeforeProcessScript and BeforeProcessStyle beidson: commit-queue-

Description Dave Hyatt 2010-06-11 11:28:13 PDT
beforeload should fire on inline scripts and stylesheets to possibly halt their processing.
Comment 1 Brian Weinstein 2010-06-11 11:39:47 PDT
<rdar://problem/8084335>
Comment 2 Brady Eidson 2010-06-11 11:41:37 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.
Comment 3 Adam Roben (:aroben) 2010-06-15 08:46:08 PDT
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
Comment 4 Brady Eidson 2010-06-15 09:09:21 PDT
(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.
Comment 5 Dave Hyatt 2010-06-21 12:31:02 PDT
Yeah I think I'm going to call it beforeprocess.
Comment 6 Brady Eidson 2010-06-25 15:55:20 PDT
Created attachment 59805 [details]
IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.
Comment 7 Darin Adler 2010-06-26 11:46:11 PDT
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.
Comment 8 Brady Eidson 2010-06-26 16:00:48 PDT
Scaffolding landed in r61964

Actually hookup coming soon.
Comment 9 Eric Seidel (no email) 2010-06-26 19:48:03 PDT
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?).
Comment 10 Brady Eidson 2010-06-26 21:37:16 PDT
There's alot more than just the HTMLScriptRunner to patch into, but I've found many places.  Patch will be up monday morning.
Comment 11 Adam Barth 2010-06-26 23:30:13 PDT
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.
Comment 12 Brady Eidson 2010-06-28 09:37:11 PDT
(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.
Comment 13 Brady Eidson 2010-06-28 16:25:39 PDT
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.
Comment 14 WebKit Review Bot 2010-06-28 16:33:34 PDT
Attachment 59958 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3357015
Comment 15 Alexey Proskuryakov 2010-06-28 17:04:21 PDT
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.
Comment 16 Brady Eidson 2010-06-28 17:27:52 PDT
(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.
Comment 17 Brady Eidson 2010-06-28 17:28:13 PDT
Especially with all the other builds continuing to work.
Comment 18 Brady Eidson 2010-06-28 17:29:32 PDT
Ahhh.  V8 vs JSC.  Great.
Comment 19 Adam Barth 2010-06-28 17:35:06 PDT
> 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.
Comment 20 Brady Eidson 2010-06-28 17:36:06 PDT
Created attachment 59963 [details]
Putting up for review just to see if Chromium's build is fixed.
Comment 21 Brady Eidson 2010-06-28 17:37:11 PDT
Okay, thanks for the heads up.

I'll land my patch that attempts to keep V8 going, but it might not quite work.
Comment 22 Brady Eidson 2010-06-28 17:37:44 PDT
(I note that at least dglazkov is CC'ed on this bug)
Comment 23 Brady Eidson 2010-06-28 17:38:54 PDT
Landed in r62052
Comment 24 WebKit Review Bot 2010-06-28 17:43:44 PDT
http://trac.webkit.org/changeset/62052 might have broken Qt Linux Release minimal
Comment 25 Brady Eidson 2010-06-28 17:44:43 PDT
(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?
Comment 26 Brady Eidson 2010-06-28 17:46:06 PDT
(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?
Comment 27 Adam Barth 2010-06-28 17:52:06 PDT
> > 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.
Comment 28 David Levin 2010-06-28 18:32:33 PDT
(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
Comment 29 Brady Eidson 2010-06-28 21:06:23 PDT
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
Comment 30 Brady Eidson 2010-06-28 21:25:48 PDT
Retitling to the effect that this bug is about script-only, and https://bugs.webkit.org/show_bug.cgi?id=41328 covers style.
Comment 31 Brady Eidson 2010-06-28 21:50:39 PDT
I have the script stuff all working, but will first attach a patch that does the "beforeProcess -> beforeProcessScript and beforeProcessStyle" divide.
Comment 32 Brady Eidson 2010-06-28 22:00:01 PDT
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
Comment 33 Eric Seidel (no email) 2010-06-28 22:02:49 PDT
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).
Comment 34 Early Warning System Bot 2010-06-28 22:11:49 PDT
Attachment 59984 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3356035
Comment 35 Sam Weinig 2010-06-28 22:13:16 PDT
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.
Comment 36 Brady Eidson 2010-06-28 22:14:34 PDT
(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.
Comment 37 Brady Eidson 2010-06-28 22:19:05 PDT
(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.
Comment 38 Adam Barth 2010-06-28 22:21:07 PDT
> 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.
Comment 39 Adam Barth 2010-06-28 22:30:10 PDT
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 40 Eric Seidel (no email) 2010-06-29 03:17:02 PDT
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 41 Eric Seidel (no email) 2010-06-29 03:17:08 PDT
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 42 Adam Roben (:aroben) 2010-06-29 07:45:51 PDT
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.
Comment 43 Eric Seidel (no email) 2010-09-02 13:57:01 PDT
Unsure if this bug still needs to be open.
Comment 44 Eric Seidel (no email) 2010-12-20 22:36:44 PST
It appears this was never landed.
Comment 45 Brady Eidson 2011-09-02 09:44:56 PDT
The reasons why we wanted this have passed.  I think Sam is nuking the code.