Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.javascript > #7966

Re: Newbie Q's: references, scopes, overwrites

Followups directed to: comp.lang.javascript

Show all headers | View raw

Dom Bannon wrote:

> Thomas 'PointedEars' Lahn wrote:
>> Dom Bannon wrote:
> 
> […] It's just a short script that does a one-off Ajax request to load in
> the other scripts:
> 
>   <script type="text/ecmascript"><![CDATA[

The proper `src' attribute value here is "text/javascript" or 
"application/javascript", with the former one being supported best.

>     setTimeout(function(){

This should be

    window.setTimeout(function() {

>       var xhr = new XMLHttpRequest;

IMO constructor calls should include the empty argument list, `()'.  If 
other environments are targeted as well, alternative code might be needed.

>       xhr.open('get','test2.svg',true);

The HTTP verb is GET, so should be the first argument of xhr.open() here.

>       xhr.onreadystatechange = function(){
>         if (xhr.readyState != 4) return;

And interesting optimization that for some reason did not occur to me until 
now (I had favored the branching code over that gauntlet).  However, what is 
missing here is checking the value of the `status' property, i. e. whether 
the request was successful (200), before attempting to process the response 
message.

>         var svg = xhr.responseXML.documentElement;
>         svg = cloneToDoc(svg); // portable importNode
>         window.svgRoot = svg;
          ^^^^^^^^^^^^^^^^
AISB, do not do that …

>         document.body.appendChild(svg);
>         delete window.svgRoot; // <- problem
          ^^^^^^^^^^^^^^^^^^^^^
… then you do not have to do that either.

>       };
>       xhr.send();

The argument list should be `(null)'.

>     },1000);
>     function cloneToDoc(node,doc){
> ...
>     }

You omitted relevant code.

>   ]]></script>
> 
>>> This works,
>>
>> It is generally unreliable.  The only way to be rather sure that new
>> script code will be executed is to append a new `script' element.
> 
> Is the 'document.body.appendChild(svg)' sufficient?

That depends on the return value of cloneToDoc(), but generally yes (with 
the provision made in my previous article).

> This is an xhtml doc, content type 'application/xhtml+xml'.

The including document?  The included document?  Both?

> The script that is loaded (ie. node 'svg') includes this code,

A markup document that has an `svg' element as its root element is not a 
script; it is either an SVG (Scalable Vector Graphics) document or an 
untyped XML document.

> which initialises everything and produces the image:
> 
> if(!window.svgRoot) {
>    svgRoot = document.documentElement;
> }
> try {
>    ...init code in one of the other svg scripts
> } catch(e) {
>    ...error report
> }
> 
> Note the test for '!window.svgRoot', despite the fact that the author
> previously did a 'delete window.svgRoot' above.

This code does not make sense.  However, you omitted relevant parts.  It 
would be best if you created and provided a minimal test case at a publicly 
accessible location.
 
>>> but only if line 5 is commented out.
>> […]
>>> If it's left in, I get various issues which have the feel of a memory
>>> over-write.
>>
>> What issues?  How did you get that impression?
> 
> The immediate error is that both F/F and Opera report that
> 'group.getBBox()' is not a function call, where 'group' is meant to be
> a group node.

As we have not seen what `group' refers to yet, that is useless, if 
generally pertinent, information at this time.

You have not explained your impression of a "memory over-write".

> Note that the svg code itself does actually work when
> viewed stand-alone (and this 'getBBox' does work correctly). Tracing
> back a bit I find that there is at least one error in
> 'svgRoot.namespaceURI'; it's set to 'w3.org/1999/xhtml', instead of
> 'w3.org/2000/svg'.

This code looks more and more bogus.
 
>>> Presumably it's still in scope, and global, and represents the same
>>> window when the other (svg) scripts are running?
>> This is not a valid question.  Please restate your request.
> 
> I wanted to find out if it was valid to reference 'window.svgRoot'
> from one of the svg scripts but, from what you say elsewhere, it looks
> like it is valid.

It is syntactically valid, but (AISB) rather unwise.
 
>>> #3: the rest of the code manipulates the svg via window.svgRoot,
>>> rather than getting a reference to the new child in the document body.
>>> Is this Ok?
>>
>> That depends on whether the object referred to by `window' already had a
>> `svgRoot' property in the first place, whether it has one after the
>> assignment in line 3, and whether the value of that property is the value
>> of `svg'.
> 
> I've just traced through the changes to 'window' with Firebug.

So your target runtime environment is Mozilla Firefox?  Which version(s)?

> 'svgRoot' doesn't exist before the assignment to 'window.svgRoot', and
> it does exist after the assignment, and it appears to be correct on a
> cursory inspection. So, the author does appear to be simply
> auto-declaring a var in 'window'.

Yes, that only appears to be so.  I have already explained what really 
happens.

>>> Presumably line 5 doesn't actually delete the new child?
>> Please restate your request.
> 
> My concern was that 'window.svgRoot' was actually a reference to the
> new child created by 'document.body.appendChild(svg)' (line #4), so
> deleting the former would actually delete the new child tree.

Deleting a property of an object usually does not remove a node from a 
document tree.  However, as host objects are involved, all bets are off.

> However, after reading up a bit more, it seems that line #4 actually
> *moves* the svg tree from 'window' to the doc body.

That would depend on what cloneToDoc() does.  If it does what its name says 
(to clone), then nothing is moved.  Certainly nothing is moved "from 
'window' to the doc body".

> So line #5 is presumably a bug anyway, unless it's intended to delete a
> var which no longer references anything.

The whole approach looks like one big bug to me at this point.

>>> #5: is there a memory overwite/leak detect tool for JavaScript?
>> Yes.
> 
> ?

<http://catb.org/~esr/faqs/smart-questions.html>


PointedEars
-- 
Prototype.js was written by people who don't know javascript for people
who don't know javascript. People who don't know javascript are not
the best source of advice on designing systems that use javascript.
  -- Richard Cornford, cljs, <f806at$ail$1$8300dec7@news.demon.co.uk>

Back to comp.lang.javascript | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Newbie Q's: references, scopes, overwrites Dom Bannon <nospam@nospam.com> - 2011-11-03 18:07 +0000
  Re: Newbie Q's: references, scopes, overwrites Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2011-11-03 20:11 +0100
    Re: Newbie Q's: references, scopes, overwrites Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2011-11-03 20:17 +0100
    Re: Newbie Q's: references, scopes, overwrites Dom Bannon <nospam@nospam.com> - 2011-11-03 20:07 +0000
      Re: Newbie Q's: references, scopes, overwrites Thomas 'PointedEars' Lahn <PointedEars@web.de> - 2011-11-03 22:33 +0100
        Re: Newbie Q's: references, scopes, overwrites Antony Scriven <adscriven@gmail.com> - 2011-11-03 19:32 -0700
  Re: Newbie Q's: references, scopes, overwrites Elegie <elegie@anonymous.invalid> - 2011-11-03 20:54 +0100

csiph-web