592872 - Web Console cleanup: NodeFactory redeclares the "factory" function

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Patrick Walton (:pcwalton)]

The NodeFactory function in HUDService.jsm looks like this:

  function NodeFactory(aFactoryType, aNameSpace, aDocument)
  {
    // aDocument is presumed to be a XULDocument
    if (aFactoryType == "text") {
      function factory(aText) {
        return aDocument.createTextNode(aText);
      }
    return factory;
  }
  else {
    if (aNameSpace == "xul") {
        function factory(aTag) {
          return aDocument.createElement(aTag);
        }
        return factory;
      }
    }
  }

Redeclaring the "factory" function like this is not actually legal in standard JavaScript. IMHO the functions should have different names.

Comment 1

[Sonny Piers [:sonny]]

The code is now:

function NodeFactory(aFactoryType, ignored, aDocument)
{
  // aDocument is presumed to be a XULDocument
  if (aFactoryType == "text") {
    return function factory(aText)
    {
      return aDocument.createTextNode(aText);
    }
  }
  else if (aFactoryType == "xul") {
    return function factory(aTag)
    {
      return aDocument.createElement(aTag);
    }
  }
  else {
    throw new Error('NodeFactory: Unknown factory type: ' + aFactoryType);
  }
}

Which is AFAIK valid.

Comment 2

[Mihai Sucan [:msucan]]

Sonny: there are two functions in the code you pasted, both are named "factory", which is not legal in standard JavaScript.

The problem is still valid in the current Web Console code.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

And even if they were named differently, function declarations in blocks should be avoided:

https://developer.mozilla.org/en/JavaScript/Guide/Functions

Comment 4

[Mihai Sucan [:msucan]]

This was most-likely fixed by bug 673148.