[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    kjs bug (onSubmit related)
From:       Michael Bayne <mdb () samskivert ! com>
Date:       2001-07-26 22:00:21
[Download RAW message or body]

So recently onSubmit wasn't doing the right thing with functions that
returned false (or true, or anything for that matter).

Then someone went in and fixed things up. The pertinent code is in
khtml/ecma/kjs_event.cpp:

void JSEventListener::handleEvent(DOM::Event &evt)
{
  if (listener.implementsCall() && win->part() ) {
    KJScript *scr = win->part()->jScript()->jScript();
    //...
    scr->call(listener, thisVal, args, *scope);
    QVariant ret = KJSOToVariant(scr->returnValue());
    //...
    if (ret.type() == QVariant::Bool && ret.toBool() == false)
        evt.preventDefault();
  }
}

This is peachy. src->call(...) results eventually in a call to
KJScriptImpl::call which contains what I think to be a bug. It looks like
this:

bool KJScriptImp::call(const KJSO &func, const KJSO &thisV,
		       const List &args, const List &extraScope)
{
  init();
  if(!func.implementsCall())
    return false;

  running++;
  recursion++;
  func.executeCall(thisV, &args, &extraScope);
  recursion--;
  running--;

  return !hadException();
}

Note however that func.executeCall(...) returns a KJSO which is the return
value from the JavaScript function and that is being ignored. Thus when
JSEventListener::handleEvent calls src->returnValue(), it's some bogus
value (whatever was there before).

I think that KJScriptImp::call probably wants to stuff that return value
into it's local return value reference, which the following diff
accomplishes:

Index: internal.cpp
===================================================================
RCS file: /home/kde/kdelibs/kjs/internal.cpp,v
retrieving revision 1.79
diff -u -p -r1.79 internal.cpp
--- internal.cpp        2001/07/23 18:17:54     1.79
+++ internal.cpp        2001/07/26 21:13:29
@@ -760,9 +760,12 @@ bool KJScriptImp::call(const KJSO &func,

   running++;
   recursion++;
-  func.executeCall(thisV, &args, &extraScope);
+  KJSO res = func.executeCall(thisV, &args, &extraScope);
   recursion--;
   running--;
+
+  // keep track of the return value
+  retVal = res.imp();

   return !hadException();
 }

I've applied this and it seems to do the right thing, but I don't know
much about reference counting KJSO instances and all that, so perhaps
something more sophisticated needs be done.

In any event, hopefully porten or pmk are reading this since they seem to
be the ones that do the most work on the kjs and ECMA stuff.

Cheers,

-- mdb /o)\ "The design as shown on television reports, however,
       \(o/  did appear to be a giant male sex organ."  -- Reuters

[prev in list] [next in list] [prev in thread] [next in thread]