Code Review: Ajax function with javascript on loaded page enabled

J

jianhua

$ cat index.html
<html>
<head>
<script src="ajax.js" type="text/javascript"></script>
</head>
<body>

<a onclick="ajax('div1', 'b.html'); return false;" href="#">
click to load page with ajax
</a>

<div id="div1"></div>

</body>
</html>
$
$ cat b.html
msg1<script type="text/javascript">alert("msg2");</script>msg3<script
type="text/javascript">alert("msg4");</script>msg5
$
$ cat ajax.js
function ajax(elmId, uri)
{
var xhr = new XMLHttpRequest();

if (!xhr) return false;
xhr.open("post", uri, true);

xhr.onreadystatechange = function(){
if (xhr.readyState == 4){
if (xhr.status == 200){
var elm = document.getElementById(elmId);
var org = xhr.responseText;
var low = xhr.responseText.toLowerCase();
var LEN = low.length;
var SCR1 = "<script";
var SCR2 = "</script>";
var pos;

for (var i = 0; i != LEN && (pos = low.indexOf(SCR1, i)) != -1;)
{
elm.innerHTML += org.substring(i, pos);
i = pos + SCR1.length;
pos = low.indexOf(">", i);
if (pos == -1) break;
i = pos + ">".length;

pos = low.indexOf(SCR2, i);
if (pos == -1) break;
var txt = document.createTextNode(
org.substring(i, pos));
i = pos + SCR2.length;
var scr = document.createElement("script");
scr.type = "text/javascript";
scr.appendChild(txt);
elm.appendChild(scr);
}
elm.innerHTML += org.substring(i);
} else {
document.getElementById(elmId).innerHTML =
xhr.status + " " + xhr.statusText;
}}}

xhr.send();
return false;
}

$
 
R

RobG

Please include some idea of why you are posting code in the body if
the message. Don't assume that those reading the body can see or
remember what the subject was.

It is much better to provide a detailed explanation of a question in
the message body than to rely on the subject alone.

$ cat index.html
<html>
Click to expand...

A valid HTML document requires a DOCTYPE.
<head>
<script src="ajax.js" type="text/javascript"></script>
</head>
Click to expand...

And a title element.
<body>

<a onclick="ajax('div1', 'b.html'); return false;" href="#">
click to load page with ajax
</a>
Click to expand...

That appears to be an inappropriate use of an A element. A better
choice would be a button.

<div id="div1"></div>

</body>
</html>
$
$ cat b.html
msg1<script type="text/javascript">alert("msg2");</script>msg3<script
type="text/javascript">alert("msg4");</script>msg5
Click to expand...

The .html file extension suggests that it contains an HTML document,
but that appears to not be so. It might be more appropriate for it to
have a .txt extension (or some other extension indicating what it
actually contains).

$
$ cat ajax.js
function ajax(elmId, uri)
{
var xhr = new XMLHttpRequest();
Click to expand...

It is not a good idea to assume support for XHMHttpRequest, a feature
test and attempts at other, equivalent functions would be better.
if (!xhr) return false;
Click to expand...

The most likely reason that !xhr will be true is that XMLHttpRequest
is not supported, in which case the script has already thrown an error
and exited.

[...]

It is much better to include a short description of what the code is
supposed to do so that it can be considered along with what the code
actually does. That will lead to an evaluation of whether the code
does it in a reasonable way or if there are alternatives that are
"better" in terms of wider support, more robust design, simplicity,
speed, and so on.
 
R

RobG

jianhua wrote:

I replied earlier but GG seems to have lost that post, my apologies if
this is a second post.

Don't assume that readers can see or remember the subject of the post.
$ cat index.html
Click to expand...

A dump of file content is usually not sufficient information for a
thorough review of anything. Supporting information is required that
states what the code is supposed to do, some criteria for evaluation
and any issues that it currently has.
<html>
<head>
<script src="ajax.js" type="text/javascript"></script>
</head>
Click to expand...

A valid HTML document requires a DOCTYPE and title element.
<body>

<a onclick="ajax('div1', 'b.html'); return false;" href="#">
click to load page with ajax
</a>
Click to expand...

A link should be used for navigation, a button would be more
appropriate here.
<div id="div1"></div>

</body>
</html>
$
$ cat b.html
msg1<script type="text/javascript">alert("msg2");</script>msg3<script
type="text/javascript">alert("msg4");</script>msg5
Click to expand...

Despite its .html extension, that file is not seem to be a valid HTML
document. Perhaps some other extension that more accurately reflects
the content would be more suitable, such as .txt.
$
$ cat ajax.js
function ajax(elmId, uri)
{
var xhr = new XMLHttpRequest();

if (!xhr) return false;
Click to expand...

To late. If the user agent doesn't support XMLHttpRequest then it has
already thrown and error and exited. Test for support for
XMLHttpRequest before attempting to use it.

[...]

Without an explanation of what the code is supposed to do, it can only
be evaluated against what it actually does. If you care to take the
time to explain what it should do and whether you see any issues in
how you are using it, you may get some useful review comments.
 
L

lovecreatesbeauty

A dump of file content is usually not sufficient information for a
thorough review of anything. Supporting information is required that
states what the code is supposed to do, some criteria for evaluation
and any issues that it currently has.
Click to expand...

Thank you.

My point was the ajax(method, elmId, url) function in my post. I
needed to write many *ajax form(s)* (is this a correct term?). The
javascript codes on the loaded pages didn't work, and I used jQuery to
solved the problem. I only used that library for that specific
purpose, so I wanted to use a single function instead of a library.

I saved a copy here: http://cyberspace.org/~jhl/ajax.js . I tested it
with latest Chrome, Internet Explorer and Firefox on Windows XP, with
latest Firefox on Fedora 15, with built-in browser on Nexus One
mobile. It works in all above environment. XMLHttpRequest is the
w3.org standard, it seems all latest browsers support it. I removed
the check on XMLHttpRequest feature.
A link should be used for navigation, a button would be more
appropriate here.
Click to expand...

It loads (can I use this term?) another page and acts like a link.
What do you think?
Despite its .html extension, that file is not seem to be a valid HTML
document. Perhaps some other extension that more accurately reflects
the content would be more suitable, such as .txt.
Click to expand...

The above file b.html will be loaded into another page, say a.html. If
there's DOCTYPE and <html> tags in the containing a.html, do I still
put DOCTYPE and <html> in the loaded file to make it more like HTML
file? It can contains a partial of a HTML file, right?

/* ajax(method, elmId, url)
* Ajax function with javascript on loaded page enabled
* jhlicc@{gmail,hotmail}.com, 20110626
*/
function ajax(method, elmId, url)
{
var xhr = new XMLHttpRequest();
var elm = document.getElementById(elmId);

xhr.onreadystatechange = function(){
if (xhr.readyState == 4){
if (xhr.status == 200){
var org = xhr.responseText;
var low = org.toLowerCase();
var LEN = org.length;
var SCR1 = "<script";
var SCR2 = "</script>";
var pos;

elm.innerHTML = "";
for (var i = 0; i != LEN && (pos = low.indexOf(SCR1, i)) != -1;)
{
elm.innerHTML += org.substring(i, pos);
i = pos + SCR1.length;
pos = low.indexOf(">", i);
if (pos == -1) break;
i = pos + ">".length;
pos = low.indexOf(SCR2, i);
if (pos == -1) break;
var scr = document.createElement("script");
scr.type = "text/javascript";
scr.text = org.substring(i, pos);
i = pos + SCR2.length;
elm.appendChild(scr);
}
elm.innerHTML += org.substring(i);
} else {
elm.innerHTML = xhr.status + " " + xhr.statusText;
}}}

xhr.open(method, url);
xhr.send();
}
 
T

Tim Streater

lovecreatesbeauty said:
Thank you.

My point was the ajax(method, elmId, url) function in my post. I
needed to write many *ajax form(s)* (is this a correct term?). The
javascript codes on the loaded pages didn't work, and I used jQuery to
solved the problem. I only used that library for that specific
purpose, so I wanted to use a single function instead of a library.

I saved a copy here: http://cyberspace.org/~jhl/ajax.js . I tested it
with latest Chrome, Internet Explorer and Firefox on Windows XP, with
latest Firefox on Fedora 15, with built-in browser on Nexus One
mobile. It works in all above environment. XMLHttpRequest is the
w3.org standard, it seems all latest browsers support it. I removed
the check on XMLHttpRequest feature.


It loads (can I use this term?) another page and acts like a link.
What do you think?
Click to expand...

You are loading part of your current page by making an ajax request,
treating the results as html and putting those results into whatever
element you choose in your page.

Don't forget to set xhr to null inside your onreadystatechange function,
when you have finished with it (i.e. once you've processed the data
returned). Otherwise the memory it occupies is not released.
 
E

Evertjan.

Stefan Weiss wrote on 01 jul 2011 in comp.lang.javascript:
Not in an external script
Click to expand...

Another reason not to use external scripts.
They don't even behave nasty.
file.
Click to expand...

Whether it is a file or another type of stream is inconsequential.
 
T

Thomas 'PointedEars' Lahn

Stefan said:
Not in an external script file.
Click to expand...

And not in a CDATA section in XHTML served as application/xhtml+xml. But
surprisingly (to me) in what is currently understood as¹ HTML5, according to
the W3C Markup Validator ("Stray end tag script.").


PointedEars
___________
¹ Didn't the WHATWG – Ian Hickson for that matter – consider such effects
from their declaring the oxymoron of a "living standard"?
 
L

lovecreatesbeauty

Don't forget to set xhr to null inside your onreadystatechange function,
when you have finished with it (i.e. once you've processed the data
returned). Otherwise the memory it occupies is not released.
Click to expand...

Thank you.

Do I use:
delete xhr;
or
xhr = null;

After xhr.send() is called, right?

I'm not sure very clear about this.

Eg. this example doesn't do that: http://jibbering.com/2002/4/httprequest.html
 
L

lovecreatesbeauty

lovecreatesbeauty wrote on 01 jul 2011 in comp.lang.javascript:


That ends javascript with some parsing engines, methinks.
Click to expand...

That one along with another "<script" are quoted. It makes them are
not tags, right?
 
T

Tim Streater

lovecreatesbeauty said:
Thank you.

Do I use:
delete xhr;
or
xhr = null;
Click to expand...

xhr = null;
After xhr.send() is called, right?
Click to expand...

No! :)

You do it after your anonymous onreadystatechange function has detected
readyState==4, and you have processed the responseText.

See here for a simple example: <http://www.clothears.org.uk>

(Note to self: must move the request = null; so it is done whatever
status value comes back.)
 
L

lovecreatesbeauty

<\/script> is not recognized as an end tag by the HTML parser. This goes
for _all_ end tags contained in JS string literals, not just </script>.

In your case, the code is in a separate JS file. The HTML parser will
never see it, so "</script>" is safe.

In XHTML documents, you can use <![CDATA[ ... ]]> to protect the
contents of the inline script (this is what Thomas was referring to).
Click to expand...

Thank you.
 
T

Thomas 'PointedEars' Lahn

Stefan said:
I can't answer that. In a "living standard", the rules can be changed at
any time. What's true today could be false tomorrow. I guess HTML5
parsers will have to retrieve and analyze the HTML5 specs every time
they want to parse a document ;)
Click to expand...

ACK, it is quite insane and not a standard (in the usual sense) at all.
<rant>
I don't know why people suddenly feel the need to get creative with
version numbers. HTML5 abandoning versioned releases, Mozilla using FF5
as a security update for FF4,... Those numbers are there for a reason!
</rant>
Click to expand...

To be fair, Firefox 5.0 actually is _not_ merely a security update.
Frankly, I do not understand why it is advertized (IIRC, even by
Mozilla.org's auto-updater) as such. Quoting the Release Notes [1],
changes include:

- Added support for CSS animations
- The Do-Not-Track header preference has been moved to increase
discoverability
- Tuned HTTP idle connection logic for increased performance
- Improved canvas, JavaScript, memory, and networking performance
- Improved standards support for HTML5, XHR, MathML, SMIL, and canvas
- Improved spell checking for some locales
- Improved desktop environment integration for Linux users
- WebGL content can no longer load cross-domain textures
- Background tabs have setTimeout and setInterval clamped to 1000ms to
improve performance
- Fixed several stability issues
- Fixed several security issues

That is security fixes 2:9 other important improvements. How is that only a
security update?

However, it beats me, too, why it had to be a new major version number.
Compare Firefox 3 with 4; it was really something new and that versioning
was acceptable. Now one could get the idea that Mozilla.org wants to keep
up with Chrome, which, after only less than 3 years of releases, is about to
reach 13.0 (and 14.0 is in the pipeline already; which would probably make
it into the Guiness World Records as the fastest software versioning ever).


PointedEars
___________
[1] <http://www.mozilla.com/en-US/firefox/5.0/releasenotes/>
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Function call fail to external script referred by page loaded with Ajax 2
Need help , Having problem with Drag n Drop ? 9
Script in AJAX-loaded page inside parent page not firing 2
Checking dynamically populated data using ajax with user entered value 5
JavaScript ajax library critique 84
Help with my responsive home page 2
How to use PDF-lib and how to center each line of texts on the page? 1
responseText 9

Members online

Total: 43 (members: 1, guests: 42)
Robots: 684

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,057
Latest member
KetoBeezACVGummies

Latest Threads

Top