Review Board 1.7.22


Array iteration cleanup in the actions_container.js file

Review Request #3873 - Created Feb. 12, 2012 and submitted

Henry Saputra
Reviewers
shindig
shindig
Some of the iterations in the actions_container.js file are using "in" operator without calling hasOwnProperty to make sure its not going up to prototype chain (http://yuiblog.com/blog/2006/09/26/for-in-intrigue/). It causes trouble when executing in some browsers like FF5.

Small cleanup for actual array to just simply used for-loop with index.
Pass JS unit tests and manual test with common container and action container.

Diff revision 2 (Latest)

1 2
1 2

  1. trunk/features/src/main/javascript/features/actions/actions_container.js: Loading...
trunk/features/src/main/javascript/features/actions/actions_container.js
Revision 1243090 New Change
[20] 67 lines
[+20] [+] function ActionRegistry() {
68
         * to paths.  This is necessary to realize actions in hierarchical
68
         * to paths.  This is necessary to realize actions in hierarchical
69
         * menus, sub-menus and drop-down toolbar buttons.
69
         * menus, sub-menus and drop-down toolbar buttons.
70
         */
70
         */
71
        var partsOfPath = path.split('/');
71
        var partsOfPath = path.split('/');
72
        var parent = this.registryByPath;
72
        var parent = this.registryByPath;
73
        for (var i in partsOfPath) {
73
        for (var i = 0; i < partsOfPath.length; i++) {
74
          var currentNode = partsOfPath[i];
74
          var currentNode = partsOfPath[i];
75
          if (!parent[currentNode]) {
75
          if (!parent[currentNode]) {
76
            parent[currentNode] = {};
76
            parent[currentNode] = {};
77
          }
77
          }
78
          parent = parent[currentNode];
78
          parent = parent[currentNode];
[+20] [20] 92 lines
[+20] function ActionRegistry() {
171
     * Returns all actions in the registry
171
     * Returns all actions in the registry
172
     */
172
     */
173
    this.getAllActions = function() {
173
    this.getAllActions = function() {
174
      var actions = [];
174
      var actions = [];
175
      for (actionId in this.registryById) {
175
      for (actionId in this.registryById) {

    
   
176
        if(this.registryById.hasOwnProperty(actionId)) {
176
        actions = actions.concat(this.registryById[actionId]);
177
          actions = actions.concat(this.registryById[actionId]);
177
      }
178
        }

    
   
179
      }
178
      return actions;
180
      return actions;
179
    };
181
    };
180

    
   
182

   
181
    /**
183
    /**
182
     * Returns all items associated with the given path
184
     * Returns all items associated with the given path
[+20] [20] 4 lines
[+20] function ActionRegistry() {
187
     */
189
     */
188
    this.getActionsByPath = function(path) {
190
    this.getActionsByPath = function(path) {
189
      var actions = [];
191
      var actions = [];
190
      var partsOfPath = path.split('/');
192
      var partsOfPath = path.split('/');
191
      var children = this.registryByPath ? this.registryByPath : {};
193
      var children = this.registryByPath ? this.registryByPath : {};
192
      for (var i in partsOfPath) {
194
      for (var i = 0; i < partsOfPath.length; i++) {
193
        var currentNode = partsOfPath[i];
195
        var currentNode = partsOfPath[i];
194
        if (children[currentNode]) {
196
        if (children[currentNode]) {
195
          children = children[currentNode];
197
          children = children[currentNode];
196
        } else {
198
        } else {
197
          // if path doesn't exist, return empty array
199
          // if path doesn't exist, return empty array
[+20] [20] 41 lines
[+20] function ActionRegistry() {
239
     *          url The gadget spec url associated with the gadget site.
241
     *          url The gadget spec url associated with the gadget site.
240
     * @param {osapi.container.GadgetSite}
242
     * @param {osapi.container.GadgetSite}
241
     *          site The instance of the gadget site.
243
     *          site The instance of the gadget site.
242
     */
244
     */
243
    this.addGadgetSite = function(url, site) {
245
    this.addGadgetSite = function(url, site) {
244
      var existingSite = this.urlToSite[url];
246
      var existingSites = this.urlToSite[url];
245
      if (existingSite) {
247
      if (existingSites) {
246
        this.urlToSite[url] = existingSite.concat(site);
248
        this.urlToSite[url] = existingSites.concat(site);
247
      } else {
249
      } else {
248
        this.urlToSite[url] = [site];
250
        this.urlToSite[url] = [site];
249
      }
251
      }
250
    };
252
    };
251

    
   
253

   
[+20] [20] 31 lines
[+20] function ActionRegistry() {
283
     * @return {Array} Always an array of the gadget site instances associated
285
     * @return {Array} Always an array of the gadget site instances associated
284
     *         with the action object.
286
     *         with the action object.
285
     */
287
     */
286
    this.getGadgetSites = function(actionId) {
288
    this.getGadgetSites = function(actionId) {
287
      var action = this.getItemById(actionId);
289
      var action = this.getItemById(actionId);

    
   
290
      var url = this.actionToUrl[actionId];

    
   
291
      var sites = [];

    
   
292
      var candidates = this.urlToSite[url];
288

    
   
293

   
289
      var url = this.actionToUrl[actionId],
294
      if (candidates) {
290
          sites,

   
291
          candidates;

   
292

    
   

   
293
      if (candidates = this.urlToSite[url]) {

   
294
        // Return subset of matching sites (gadget view matches declared action view,
295
        // Return subset of matching sites (gadget view matches declared action view,
295
        // if the action declared a view) Do not modify existing array.
296
        // if the action declared a view) Do not modify existing array.
296
        for (var i = 0, site; site = candidates[i]; i++) {
297
        for (var i = 0; i < candidates.length; i++) {

    
   
298
          var site = candidates[i];
297
          var holder = site.getActiveSiteHolder();
299
          var holder = site.getActiveSiteHolder();
298
          if (!action.view || (holder && holder.getView() === action.view)) {
300
          if (!action.view || (holder && holder.getView() === action.view)) {
299
            (sites = sites || []).push(site);
301
            sites.push(site);
300
          }
302
          }
301
        }
303
        }
302
      }
304
      }
303

    
   
305

   
304
      return sites;
306
      return sites;
[+20] [20] 91 lines
[+20] [+] function addAction(actionObj, url) {
396
    // Comply with spec by passing an array of the object
398
    // Comply with spec by passing an array of the object
397
    // TODO: Update spec, since there will never be more than 1 element in the array
399
    // TODO: Update spec, since there will never be more than 1 element in the array
398
    showActionHandler([actionObj]);  // notify the container to display the action
400
    showActionHandler([actionObj]);  // notify the container to display the action
399

    
   
401

   
400
    for (var to in showActionSiteIds) {
402
    for (var to in showActionSiteIds) {

    
   
403
      if(showActionSiteIds.hasOwnProperty(to)) {
401
      if (!container_.getGadgetSiteByIframeId_(to)) {
404
        if (!container_.getGadgetSiteByIframeId_(to)) {
402
        delete showActionSiteIds[to];
405
          delete showActionSiteIds[to];
403
      }
406
        }
404
      else {
407
        else {
405
        // Comply with spec by passing an array of the object
408
          // Comply with spec by passing an array of the object
406
        // TODO: Update spec, since there will never be more than 1 element in the array
409
          // TODO: Update spec, since there will never be more than 1 element in the array
407
        gadgets.rpc.call(to, 'actions.onActionShow', null, [actionObj]);
410
          gadgets.rpc.call(to, 'actions.onActionShow', null, [actionObj]);
408
      }
411
        }
409
    }
412
      }

    
   
413
    }
410
  };
414
  };
411

    
   
415

   
412
  /**
416
  /**
413
   * Removes the action from the action registry, and removes the action from
417
   * Removes the action from the action registry, and removes the action from
414
   * the container UI.
418
   * the container UI.
[+20] [20] 9 lines
[+20] [+] function removeAction(id) {
424
    // Comply with spec by passing an array of the object
428
    // Comply with spec by passing an array of the object
425
    // TODO: Update spec, since there will never be more than 1 element in the array
429
    // TODO: Update spec, since there will never be more than 1 element in the array
426
    hideActionHandler([actionObj]);  // notify the container to hide the action
430
    hideActionHandler([actionObj]);  // notify the container to hide the action
427

    
   
431

   
428
    for (var to in hideActionSiteIds) {
432
    for (var to in hideActionSiteIds) {

    
   
433
      if (hideActionSiteIds.hasOwnProperty(to)) {
429
      if (!container_.getGadgetSiteByIframeId_(to)) {
434
        if (!container_.getGadgetSiteByIframeId_(to)) {
430
        delete hideActionSiteIds[to];
435
          delete hideActionSiteIds[to];
431
      }
436
        }
432
      else {
437
        else {
433
        // Comply with spec by passing an array of the object
438
          // Comply with spec by passing an array of the object
434
        // TODO: Update spec, since there will never be more than 1 element in the array
439
          // TODO: Update spec, since there will never be more than 1 element in the array
435
        gadgets.rpc.call(to, 'actions.onActionHide', null, [actionObj]);
440
          gadgets.rpc.call(to, 'actions.onActionHide', null, [actionObj]);
436
      }
441
        }
437
    }
442
      }

    
   
443
    }
438
  };
444
  };
439

    
   
445

   
440
  /**
446
  /**
441
   * A map of all listeners.
447
   * A map of all listeners.
442
   *
448
   *
[+20] [20] 22 lines
[+20] function removeAction(id) {
465
    if (!selection && container_ && container_.selection) {
471
    if (!selection && container_ && container_.selection) {
466
      selection = container_.selection.getSelection();
472
      selection = container_.selection.getSelection();
467
    }
473
    }
468

    
   
474

   
469
    // call all container listeners, if any, for this actionId
475
    // call all container listeners, if any, for this actionId
470
    var list = actionListenerMap[id];
476
    var listenersArray = actionListenerMap[id];
471
    if (list) {
477
    var i;
472
      for (var i = 0, listener; listener = list[i]; i++) {
478
    if (listenersArray) {

    
   
479
      for (i = 0; i < listenersArray.length; i++) {

    
   
480
        var listener = listenersArray[i];
473
        listener.call(null, id, selection);
481
        listener.call(null, id, selection);
474
      }
482
      }
475
    }
483
    }
476
    for (var i = 0, listener; listener = actionListeners[i]; i++) {
484
    for (i = 0;  i < actionListeners.length; i++) {

    
   
485
      var listener = actionListeners[i];
477
      listener.call(null, id, selection);
486
      listener.call(null, id, selection);
478
    }
487
    }
479

    
   
488

   
480
    // make rpc call to get gadgets to run callback based on action id
489
    // make rpc call to get gadgets to run callback based on action id
481
    var gadgetSites = registry.getGadgetSites(id);
490
    var gadgetSites = registry.getGadgetSites(id);
482
    if (gadgetSites) {
491
    if (gadgetSites) {
483
      for (var i = 0, site; site = gadgetSites[i]; i++) {
492
      for (i = 0; i < gadgetSites.length; i++) {

    
   
493
        var site = gadgetSites[i];
484
        var holder = site.getActiveSiteHolder();
494
        var holder = site.getActiveSiteHolder();
485
        if (holder) {
495
        if (holder) {
486
          gadgets.rpc.call(holder.getIframeId(), 'actions.runAction', null,
496
          gadgets.rpc.call(holder.getIframeId(), 'actions.runAction', null, id, selection);
487
            id, selection

   
488
          );

   
489
        }
497
        }
490
      }
498
      }
491
    }
499
    }
492
  };
500
  };
493

    
   
501

   
494
  /**
502
  /**
495
   * Callback for loading actions after gadget has been preloaded.
503
   * Callback for loading actions after gadget has been preloaded.
496
   *
504
   *
497
   * @param {Array}
505
   * @param {Object}
498
   *          Response from container's lifecycle handling of preloading the
506
   *          Response from container's lifecycle handling of preloading the
499
   *          gadget.
507
   *          gadget.
500
   */
508
   */
501
  var preloadCallback = function(response) {
509
  var preloadCallback = function(response) {
502
    for (var url in response) {
510
    for (var url in response) {

    
   
511
      if(!response.hasOwnProperty(url)) {

    
   
512
        continue;

    
   
513
      }
503
      var metadata = response[url];
514
      var metadata = response[url];
504
      if (metadata.error || !metadata.modulePrefs) {
515
      if (metadata.error || !metadata.modulePrefs) {
505
        continue; // bail
516
        continue; // bail
506
      }
517
      }
507

    
   
518

   
[+20] [20] 13 lines
[+20] var preloadCallback = function(response) {
521
      if (!actionsJson) {
532
      if (!actionsJson) {
522
        continue; // bail
533
        continue; // bail
523
      }
534
      }
524

    
   
535

   
525
      var actions = [].concat(actionsJson['action']);
536
      var actions = [].concat(actionsJson['action']);
526
      for (var i = 0, actionObj; actionObj = actions[i]; i++) {
537
      for (var i = 0; i < actions.length; i++) {

    
   
538
        var actionObj = actions[i];
527
        var actionClone = {};
539
        var actionClone = {};
528
        // replace @ for attribute keys;
540
        // replace @ for attribute keys;
529
        for (var key in actionObj) {
541
        for (var key in actionObj) {

    
   
542
          if(actionObj.hasOwnProperty(key)) {
530
          actionClone[key.substring(1)] = actionObj[key];
543
            actionClone[key.substring(1)] = actionObj[key];
531
        }
544
          }

    
   
545
        }
532
        // check if action already exists
546
        // check if action already exists
533
        if (!registry.getItemById(actionClone.id)) {
547
        if (!registry.getItemById(actionClone.id)) {
534
          addAction(actionClone, url);
548
          addAction(actionClone, url);
535
        }
549
        }
536
      }
550
      }
[+20] [20] 31 lines
[+20] [+] var closedCallback = function(site) {
568
   * @param {String}
582
   * @param {String}
569
   *          Gadget spec url for the gadget that has been unloaded.
583
   *          Gadget spec url for the gadget that has been unloaded.
570
   */
584
   */
571
  var unloadedCallback = function(url) {
585
  var unloadedCallback = function(url) {
572
    var actionsForUrl = registry.getActionsByUrl(url);
586
    var actionsForUrl = registry.getActionsByUrl(url);
573
    for (var i in actionsForUrl) {
587
    for (var i = 0; i < actionsForUrl.length; i++) {
574
      var action = actionsForUrl[i];
588
      var action = actionsForUrl[i];
575
      removeAction(action.id);
589
      removeAction(action.id);
576
    }
590
    }
577
  };
591
  };
578

    
   
592

   
[+20] [20] 249 lines
  1. trunk/features/src/main/javascript/features/actions/actions_container.js: Loading...