mkuklis / phonegap-websocket

Websocket PhoneGap plugin for Android
203 stars 78 forks source link

FIX dispatchEvent: call eventHandler in context of the WebSocket #50

Closed russaa closed 9 years ago

russaa commented 9 years ago

small fix in function dispatchEvent(): call the event handlers in context of the WebSocket instance (instead of global/window context)

mkuklis commented 9 years ago

@russaa thanks for the pull request. Unfortunately I don't think this is how the original WebSocket spec handles the context. I think all callbacks execute in the global scope.

russaa commented 9 years ago

I have to admit, I did not look at the specification... but doesn't the general event-handling routine (for HTML/DOM elements) apply here too?

As far as I can make out the WebSocket spec. the event handler specification applies: there in step 10, the Lexical Environment Scope, sub-point 3 suggests that the scope would be the element i.e. the WebSocket object (?): "3. If element is not null, let Scope be the result of NewObjectEnvironment(element, Scope)."

Also (from a practical point of view), when I use WebSockets in Chrome or Firefox the handlers get executed within the context of the WebSocket instance, e.g. using the little test-script below, I get the following results (note the [object WebSocket] where the this reference is printed):

event OPEN: ...execution context is [object WebSocket]
ONOPEN: ...execution context is [object WebSocket]
ONMESSAGE: ...execution context is [object WebSocket]  URL: ws://echo.websocket.org/
event MESSAGE: ...execution context is [object WebSocket]
event MESSAGE2: ...execution context is [object WebSocket]  URL: ws://echo.websocket.org/
  ...
event CLOSED: ...execution context is [object WebSocket]

test-script

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">
  <head>
    <meta http-equiv="content-type" content="text/html; charset=utf-8"/>
    <title>JavaScript Testing: WebSocket execution context of handlers</title>
  </head>

  <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.8.2/jquery.min.js"></script>
  <script type="text/javascript">

  $(function(){
    testWebSocketExecContext();
  });

  var print = function(msg){
    $('#text-content').text($('#text-content').text()+'\r\n'+msg);
  };

  var testWebSocketExecContext = function(){

    var url = 'ws://echo.websocket.org';
    var s = new WebSocket(url);
    var REF = 'theSocket';

    var testScript = [
        'start ', 'stop', 'analyze '
    ];
    var testLoops = 2;
    var reqId = 1;
    var runTest = function(i){

        if(i >= testScript.length){

            if(reqId >= testLoops){

                console.log('>> closing...');
                s.close()

                return;
            }

            ++reqId;
            i=0;
        }

        var test = testScript[i];
        if(/\s$/ig.test(test)) test += reqId;

        console.log('>> sending req msg "'+test+'"...');

        s.send(test);

        setTimeout(function(){ runTest(++i); }, 1000);
    };

    s.onmessage = function(data){
       console.log('ONMESSAGE: received message "'+data+'"');
       print('ONMESSAGE: ...execution context is '+this+'  URL: '+(this.url));
    };

    s.onerror = function(err){
       console.error('ONERROR occured "'+err+'"');

       print('ONERROR: ...execution context is '+this+'  URL: '+(this.url));
    };

    s.onclose = function(code){
       console.info('ONCLOSE invoked "'+code+'"');

       print('ONCLOSE: ...execution context is '+this+'  URL: '+(this.url));
    };

    s.addEventListener('message', function(data){
       console.log('event MESSAGE: received message "'+data+'"');

       print('event MESSAGE: ...execution context is '+this);
    });

    s.addEventListener('message', function(data){
       console.log('event MESSAGE2: received message "'+data+'"');
       print('event MESSAGE2: ...execution context is '+this+'  URL: '+this.url);
    });

    s.addEventListener('error', function(data){
       console.log('event ERROR: received message "'+data+'"');
       print('event ERROR: ...execution context is '+this);
    });

    s.addEventListener('close', function(data){
       console.info('event CLOSED: received message "'+data+'"');
       print('event CLOSED: ...execution context is '+this);
    });

    s.addEventListener('open', function(data){
       print('event OPEN: ...execution context is '+this);
    });

    s.onopen = function(){

        print('ONOPEN: ...execution context is '+this);

        setTimeout(function(){
            runTest(0);
        }, 500);

    };

  };
  </script>

  <body>

  <div>
  <pre id="text-content"></pre>
  </div>

 </body>
 </html>
mkuklis commented 9 years ago

@russaa you are right. I tested it too but I was doing it wrong. Thanks again for the fix!