Bug 6078

Summary: KJSProxy (and other KDE legacy code) should be removed
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: WebKit Misc.Assignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Proposed patch (just removes code)
darin: review-
Newer patch (no more SVGEventListener or interpreterForGlobalObject changes) darin: review+

Description Eric Seidel (no email) 2005-12-14 03:26:51 PST
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.
Comment 1 Eric Seidel (no email) 2005-12-14 05:02:17 PST
Created attachment 5077 [details]
Proposed patch (just removes code)
Comment 2 Eric Seidel (no email) 2005-12-14 05:04:45 PST
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.
Comment 3 Darin Adler 2005-12-14 09:42:35 PST
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.
Comment 4 Darin Adler 2005-12-14 09:43:00 PST
Comment on attachment 5077 [details]
Proposed patch (just removes code)

Marking review- because removing interpreterForGlobalObject is wrong.
Comment 5 Geoffrey Garen 2005-12-14 10:00:42 PST
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?
Comment 6 Eric Seidel (no email) 2005-12-14 10:39:50 PST
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.
Comment 7 Eric Seidel (no email) 2005-12-14 12:10:22 PST
Created attachment 5080 [details]
Newer patch (no more SVGEventListener or interpreterForGlobalObject changes)
Comment 8 Darin Adler 2005-12-14 12:38:21 PST
Comment on attachment 5080 [details]
Newer patch (no more SVGEventListener or interpreterForGlobalObject changes)

Looks fine. r=me