Bug 6078 – KJSProxy (and other KDE legacy code) should be removed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Eric Seidel (no email)]

KJSProxy should be removed

Basically this is just a big cleanup patch, which removes a bunch of now un-used khtml stuff.  We've long 
sinced forked from KHTML, this is just one of many patches needed to make our code faster and cleaner 
at the cost of further breaking "compatibility" with khtml.

[Eric Seidel (no email)]

Created attachment 5077 [details]
Proposed patch (just removes code)

[Eric Seidel (no email)]

Comment on attachment 5077 [details]
Proposed patch (just removes code)

The patch is unfortunately a bit long, however all it does is remove code so it
should be very straightforward to read through.  I killed KJSProxy (replacing
it with KJSProxyImpl) and removed KLibrary, KGlobal, KInstance, fixed
KStandardDirectories::locate to actually be able to load other things besides
khtml4.css and quirks.css (means that svg.css will now load).  Basically any
code that I touched which contained code which should be removed, I removed.

[Darin Adler]

Comment on attachment 5077 [details]
Proposed patch (just removes code)

Do *not* return interpreterForGlobalObject. That will break LiveConnect and
Objective-C bindings. I'll read over the rest of the patch later.

[Darin Adler]

Comment on attachment 5077 [details]
Proposed patch (just removes code)

Marking review- because removing interpreterForGlobalObject is wrong.

[Geoffrey Garen]

This patch adds SVGEventListener to the project file. Why?

Can you get rid of this line too? :
     settings = view ? view->part()->settings() : 0;

It would be nice if you wrote a test for the new locate() function. Or would its failure just break everything?

[Eric Seidel (no email)]

Hum... I did finally find where interpreterForGlobalObject was used (in kjs/bindings/*).  I'll add it back.

SVGEventListener is a *very simple* subclass of JSLazyEventListener which I wrote, but decided I would 
attach to a different patch.  I'll make sure to land them near each other.

settings = view ? view->part()->settings() : 0;

Nope, settings is used later in that class... unless I just pull the settings off of the view/part when their 
needed, which I could do.

Yes, the new locate did break everything (litterally everything) when I got it wrong the first time.  Also, 
now loading svg.css changes the way at least a couple SVG tests are rendered, so I think its covered.

[Eric Seidel (no email)]

Created attachment 5080 [details]
Newer patch (no more SVGEventListener or interpreterForGlobalObject changes)

[Darin Adler]

Comment on attachment 5080 [details]
Newer patch (no more SVGEventListener or interpreterForGlobalObject changes)

Looks fine. r=me