Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:27674
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain googlemail.com designates 64.233.182.186 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=googlemail.com; s=beta;
        h=received:message-id:date:from:user-agent:mime-version:cc:subject:content-type:content-transfer-encoding;
        b=RnpAHjFHVzt9A9ySkin1thZ2vdcJsD4FbOadTck2CDk6P/n8+kKTojpdTMFZg4iAFS2qAsAooF76o5+Q2PnnZRFps9njYthBSdG4nJBU1rrcw1Bf2boA9jagOqu0U038N7j+8Td+FHIurnEKS+kbgicC2QcpiBj+7CjMepL2DHA=
Message-ID: <45BA2E9E.9040907@googlemail.com>
Date: Fri, 26 Jan 2007 16:38:54 +0000
User-Agent: Thunderbird 1.5.0.9 (Windows/20061207)
MIME-Version: 1.0
CC: PHP internals list <internals@lists.php.net>, 
 Wez Furlong <kingwez@gmail.com>,
 "Frank M. Kromann" <frank@kromann.info>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: COM extension defects: Patch for defect 37927
From: andy.wharmby@googlemail.com (Andy Wharmby)

Hi Internals,
       Attached are details of a suggested patch for COM extension 
defect 37927 (http://bugs.php.net/bug.php?id=37927).
All comments welcome; good or bad.

Regards
    Andy

Andy Wharmby
IBM United Kingdom Limited
Winchester, England SO21 2JN
E-mail: andy_wharmby@uk.ibm.com

_*COM Defect 37927*_

This defect actually reports issues with 2 different event sink 
interfaces, DwebBrowserEvents2 and DWebbrowserEvents. So taking each
one in turn:

*DWebBrowserEvents2::NewWindow2 Event  *
This event is documented  at : 
http://msdn.microsoft.com/workshop/browser/webbrowser/reference/ifaces/dwebbrowserevents2/newwindow2.asp
The defect details an issue with the NewWindow2 event handler but my 
testing has shown the exact same issue with NewWindows3 event too.

The problem here is that the user defined event handler is not getting 
called when the associated event occurs in IE and my investigations have 
shown this
 is due to a bug in the COM extension code. Examining the COM trace 
output with DebugView shows:

[4716] T=00001890
[4716]  PHP:IEEventSinker InvokeEx
[4716] T=00001890
[4716] -- Invoke: 251           newwindow2 [10] flags=00000001 args=2
[4716] T=00001890
[4716] alloc zval for arg 0 VT=00004009  << only one of two arguemets 
gets processed !!
[4716] T=00001890
[4716]  PHP:IEEventSinker InvokeEx  <<next event

So the event is reported but is only partially processed by the COM 
extension code; no attempt is made to call the event handler.

BTW. This  COM trace in com_wrapper.c, which output using 
"OutputDebugString", is active in non-DEBBUG builds unlike all other 
uses in the PHP code base.
I am not sure what the cost of these calls is but I would have expected 
such calls to be enabled in DEBUG builds only. Easily fixed and I will 
put together
a fix unless anyone sees a need for this trace in non-DEBUG, i.e 
production builds.

Anyway the reason for the partial request processing is because the code 
is trapping in php_com_wrap_variant() when processing the input 
arguments for the
"NewWindow2" (and NewWindow3) event.  The NewWIndow2 event is defined on 
MSDN as follows:

void NewWindow2(IDispatch **&ppDisp, VARIANT_BOOL *&Cancel);

i.e a double indirect pointer to a IDispatch. However, this argument is 
defined for output. The VT_DISPATCH variant addressed has its type set but
its value is NULL , i.e V_DISPATCH(<var addr>) == NULL  . So when 
php_com_wrap_variant() attempts a  call on the IDispatch interface to 
get type
information and the PHP thread traps. Easily fixed by checking for a 
NULL IDispatch interface pointer prior to call.

Required patch to fix is here: http://www.pastebin.ca/329136

This fixes the problem of the event handler not being called in the 
first place. However any attempt by a NewWindow2/3 event handler  to 
cancel the navigation,
i.e  by setting  $cancel == TRUE,  will still fail until the fix for 
defect  345764 is dropped (http://bugs.php.net/bug.php?id=34564). Frank 
is currently reviewing this fix.

*DWebBrowserEvents::NewWindow Event  *
This event is documented  at : 
http://msdn.microsoft.com/workshop/browser/webbrowser/reference/ifaces/dwebbrowserevents/newwindow.asp.

However, the MSDN description does include the following warning:

    "This interface is obsolete; use the DWebBrowserEvents2 interface 
instead. The DWebBrowserEvents2 interface provides more
     control over the WebBrowser control than the DWebBrowserEvents 
interface."

 From my investigations I believe this interface has fallen into a state 
of disrepair and should not be used anymore. I can see no reason why 
anyone would
need to use DWebBrowserEvents in preference to  DWebBrowserEvents2; 
given that the minimum requirements, IE and Windows levels,
are the same in both cases.

During my investigation I found 2 problems which appear IE related and 
not issues in the COM extension itself:

(1) The interface defines NewWindow as an event that takes just 2 
arguments. However it appears it actually now takes 6 !!.

The COM trace shows: 

[7060] T=00001de4
[7060]  PHP:IEEventSinker InvokeEx
[7060] T=00001de4
[7060] -- Invoke: 107            newwindow [9] flags=00000001 args=6
[7060] T=00001de4
[7060] alloc zval for arg 0 VT=00000008
[7060] T=00001de4
[7060] alloc zval for arg 1 VT=00000003
[7060] T=00001de4
[7060] alloc zval for arg 2 VT=00000008
[7060] T=00001de4
[7060] alloc zval for arg 3 VT=0000400c
[7060] T=00001de4
[7060] alloc zval for arg 4 VT=00000008
[7060] T=00001de4
[7060] alloc zval for arg 5 VT=0000400b
[7060] T=00001de4
[7060] arguments processed, prepare to do some work
[7060] T=00001de4
[7060] function called ok

So defining the event handler as described on the MSDN site will result 
in unpredictable behaviour; certainly navigation will not be prevented
with the supplied testcase.

The 6th argument is of type VT_VOOL so I am guessing that's the "cancel" 
argument. By adding dummy arguments to the event handler as follows:

 function NewWindow(&$dum1, $dum2, $dum3, $dum4, $dum5, &$Cancel) {
    $this->newWindowOpened = true;
    echo "NewWindow event was fired.\n";
    variant_set($Cancel,true);
    return;
  }

and with the fix for defect  345764 applied then the NewWindow event is 
fired the first time its tried and navigation is cancelled. However any 
subsequent attempt to
open any link in a new window is also prevented even though not all 
attempts result in a NewWindow event being reported and occasionally I 
am unable to close
down IE so perhaps my guess at what the 6th argument does is invalid!!. 
Everything is fine when DWebBrowserEvents2 interface is used.

(2) The supplied testcase defines the "OnQuit" event which is, according 
to MSDN, defined by the DWebBrowserEvents interface. Alas not !! Or at 
least not using IE
 Version 6. The actual event name appears to be  "Quit" and not 
"OnQuit". Again here is the COM trace of the event reported using the 
supplied test case:

[6012]  PHP:IEEventSinker InvokeEx
[6012] T=00000ff0
[6012] -- Invoke: 103                 quit [4] flags=00000001 args=1
[6012] T=00000ff0
[6012] alloc zval for arg 0 VT=0000000b
[6012] T=00000ff0
[6012] arguments processed, prepare to do some work
[6012] T=00000ff0
[6012] failed to call func

This clearly shows the event is named "quit" and so the call to the any 
handler named "onQuit" fails. Modifying the testcase to define a method 
"Quit" and the user
defined method fires OK.