arvgta / ajaxify

Ajaxify - An Ajax Plugin
https://4nf.org/
275 stars 124 forks source link

XHTML javascript breaks Ajaxify #41

Closed Shade- closed 9 years ago

Shade- commented 9 years ago

Hi, I've experienced an "Uncaught SyntaxError: Unexpected token <" error using Ajaxify in a project I'm building. No demo is available as it's a secret one, but I have found that the issue is due to XHTML-designed javascript:

<script type="text/javascript">
<!--
    document.write('<div></div>');
-->
</script>

The HTML commenting parts cause an error when parsing the inline script, breaking its syntax.

arvgta commented 9 years ago

Hi Shade,

Thanks for raising this issue!

On a side note, it's not a good idea to use

document.write()

...in an Ajax environment!

I won't fix this one, because Ajaxify is geared at modern browsers. Please see also:

http://stackoverflow.com/questions/1507939/using-html-comment-tag-still-relevant-around-javascript-code

What is it you'd like to do? Comment out the code within the script tags? I reckon you could do it like this:

<script type="text/javascript">
 // document.write('<div></div>');
</script>

Thanks and Regards

Shade- commented 9 years ago

I know document.write should not be used in an AJAX environment, but I'm trying to integrate Ajaxify with an existing project which has numerous fallbacks to non-javascript users involving document.write. Concerning the HTML tags in scripts, I would like to strip them out using Javascript: can you point out where exactly the code is parsed, and where I could remove them in Ajaxify? I wouldn't mind reverse-engineering the entire code but since I've got a very reduced amount of spare time, it would take me a lot.

Also, another issue I've encountered: upon submitting a form, scripts and stylesheets are doubled, fired and the new ones are not loaded. Since I use the same scripts and stylesheets when the user is logged in, I've spotted this trying to build a nice AJAX login page, but I believe this is common to all forms. After some intense debugging, I've found that the following line causes this bug:

if (same) return _sameScripts($scriptsN, PK);

"same" var is populated with the form POST data, not with false, null or true which should be the intended content for it. This causes _sameScripts() function to be executed instead of outputting the new content and removing the old one. Delta option is enabled.

I've uncommented that line and it works just fine, but I believe you as the author should have the last word on the problem.

arvgta commented 9 years ago

Just some quick information so you can continue with your project - here's the code that processes an inline script (passed as "t")

lines 345-355

function _addtext(t) {
            try {
                $.globalEval(t);
            } catch (e1) {
                try { 
                    eval(t);
                } catch (e2) {
                    $.log("Error in inline script : " + t + "\nError code : " + e2);
                }
            }
        }

Thanks for pointing out the bug in form handling! I suppose you're changing the page on POST? (e.g. after login)

I reckon the current implementation of forms only works if the page remains the same...

I'll look into that asap!

Shade- commented 9 years ago

Thank you!

Yes indeed, the page changes on POST. That should be a valid patch though.

arvgta commented 9 years ago

Thank you, too!

Do you mean patching it with removing the line:

if (same) return _sameScripts($scriptsN, PK);

?

Problem is, that I have to cater for both cases:

1) Page stays the same 2) Page changes

(If the page stays the same, the above line is needed!)

The patch you proposed might work for your project, but probably only if your page never stays the same in the form handling.

So I probably have to find a more general patch...

Thanks very much!

Shade- commented 9 years ago

If page changes, another call to _sameScripts() is performed later on. You should really check for "same" var, as it can have unexpected values other than booleans or null, and this I guess it's the clue part.

arvgta commented 9 years ago

I agree: The "same" value should hold a boolean or null. I think I have to make sure that is the case within _ajaxify_forms or this line in _request

fn(href, reqr, post); //Call "fn" - handler of parent, informing whether POST or not
Shade- commented 9 years ago

Another little thing: it seems like inline scripts are eval()'d before "src" ones in some cases (eg.: when one or more "src" scripts are particularly heavy). Is there a way to wait for "src" scripts to load completely, and just then load inline scripts? Eg. I'd like to load CodeMirror for a particular textarea, but the inline script pops up an error because CodeMirror is not defined (actually, it is defined but slightly after the inline script is ran). I know I might include a check for CodeMirror typeof but it would not load anyway. I can't really tell where the inline scripts and the "src" ones are parsed in the code, although I have done some debugging with not so fortunate results. Would you be my guide throughout your code? :)

arvgta commented 9 years ago

Here's the salient code:

        function _lSel(p, $t) { //load page into DOM and handle scripts
            pass++;
            _lDivs($t);
            $.scripts(p);
            $.scripts("s");
            $.scripts("a");
            return $.scripts("c");
        }

So in pseudo-code:

1) Load in the content divs 2) Delta-loading (including the "src" ones) 3) Style tags 4) Inline scripts 5) Return canonical- value

So the "src" ones are handled beforehand...

arvgta commented 9 years ago

Made these changes

Please test this file@4nf.org

or

this one

Thanks!

Shade- commented 9 years ago

It works fine! Thank you! I have another question but I do think it's better to open a new issue for that.