You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Ganesh Jung (JIRA)" <de...@myfaces.apache.org> on 2010/03/04 10:36:27 UTC

[jira] Created: (MYFACES-2586) wrong calling order for ajax script autorun

wrong calling order for ajax script autorun
-------------------------------------------

                 Key: MYFACES-2586
                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
             Project: MyFaces Core
          Issue Type: Bug
          Components: JSR-314
    Affects Versions: 2.0.0-beta-2
         Environment: Javascript
            Reporter: Ganesh Jung


First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:

	    	<h:inputText value="#{myBean.test}">
				<f:ajax render="test" />
	    	</h:inputText>
	    	<h:panelGroup id="test">
			    <script type="text/javascript">
					alert("running");
				</script>
		    	<h:inputText value="#{myBean.test}" />
	    	</h:panelGroup>

Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841308#action_12841308 ] 

Werner Punz edited comment on MYFACES-2586 at 3/4/10 2:58 PM:
--------------------------------------------------------------

Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible this was introduced.

The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
The issue as I see here is that we have probably double ids for a fraction of a second which might collide with document.getElementById() if done inlined.





      was (Author: werpu):
    Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible I would not see this as a bug.
The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
The issue as I see here is that we have probably double ids for a fraction of a second which might collide with document.getElementById() if done inlined.




  
> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841308#action_12841308 ] 

Werner Punz commented on MYFACES-2586:
--------------------------------------

Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible I would not see this as a bug.
The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
I personally would say we have to leave it as it is, since this is more or less just a cosmetic issue instead of anything else.
I rather have a similar rendering behavior over all platforms instead of a better one on some which might break things over 
other browsers.



> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Werner Punz resolved MYFACES-2586.
----------------------------------

    Resolution: Fixed

done works, reasonably well now, and the problem of the double ided elements for a fraction of a second now is gone


> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841308#action_12841308 ] 

Werner Punz edited comment on MYFACES-2586 at 3/4/10 3:06 PM:
--------------------------------------------------------------

Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible this was introduced.

The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.


      was (Author: werpu):
    Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible this was introduced.

The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
The issue as I see here is that we have probably double ids for a fraction of a second which might collide with document.getElementById() if done inlined.




  
> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841289#action_12841289 ] 

Werner Punz commented on MYFACES-2586:
--------------------------------------

I will check that one tomorrow...

> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841331#action_12841331 ] 

Werner Punz edited comment on MYFACES-2586 at 3/4/10 3:50 PM:
--------------------------------------------------------------

Ok I think I have a solution, but I need a second eye to check it before commit the insert code now should be as follows to trigger the right order:
if (newTag != "") {
                var evalNode = null;
                if (typeof window.Range != 'undefined'
                        && typeof Range.prototype.createContextualFragment == 'function') {
                    var range = document.createRange();
                    range.setStartBefore(item);
                    var fragment = range.createContextualFragment(newTag);
                    evalNode = item.parentNode.replaceChild(fragment, item);
                } else {
                    item.insertAdjacentHTML('beforeBegin', newTag);
                    evalNode = item.previousSibling;
                    item.parentNode.removeChild(item);
                }

                // and remove the old item
                //first we have to save the node newly insert for easier access in our eval part
                if (myfaces._impl._util._Utils.isManualScriptEval()) {
                    myfaces._impl._util._Utils.runScripts(request, context, evalNode);
                }
                return;
            }
            // and remove the old item, in case of an empty newtag and do nothing else
            item.parentNode.removeChild(item);

We get a slightly different behavior on IE6 and ie7 due to having to revert to insertAdjacentHTML due to lack of W3C range objects. (We should rethink
the usage of innerHTML for those browsers to get the same behavior), but since neither ie6 and ie7 do any auto eval I can live with a slightly different render behavior on those browsers for now. 
Anyway I gave it a testing on ie8 ie8 compatbilitymode ie7 mozilla, safari and opera and so far the solution works out quite well, in case of auto eval the problematic double dom entries are removed and in case of ie the eval is triggered after the dom is in a clean state. I guess I will commit the fix.


      was (Author: werpu):
    Ok I think I have a solution, but I need a second eye to check it before commit the insert code now should be as follows to trigger the right order:
if (newTag != "") {
                var evalNode = null;
                if (typeof window.Range != 'undefined'
                        && typeof Range.prototype.createContextualFragment == 'function') {
                    var range = document.createRange();
                    range.setStartBefore(item);
                    var fragment = range.createContextualFragment(newTag);
                    evalNode = item.parentNode.replaceChild(fragment, item);
                } else {
                    item.insertAdjacentHTML('beforeBegin', newTag);
                    evalNode = item.previousSibling;
                    item.parentNode.removeChild(item);
                }

                // and remove the old item
                //first we have to save the node newly insert for easier access in our eval part
                if (myfaces._impl._util._Utils.isManualScriptEval()) {
                    myfaces._impl._util._Utils.runScripts(request, context, evalNode);
                }
                return;
            }
            // and remove the old item, in case of an empty newtag and do nothing else
            item.parentNode.removeChild(item);

We get a slightly different behavior on IE6 and ie7 due to having to revert to insertAdjacentHTML due to lack of W3C range objects. (We should rethink
the usage of innerHTML for those browsers to get the same behavior), but since neither ie6 and ie7 do any auto eval I can live with a slightly different render behavior on those browsers for now. 

Anyway please have a look at this and as soon as everyone agrees I can commit this.

  
> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Ganesh Jung (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841448#action_12841448 ] 

Ganesh Jung commented on MYFACES-2586:
--------------------------------------

Great Werner, thank you. I would have worked on a fix next weekend, but now I'm happy it's done!

> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841308#action_12841308 ] 

Werner Punz edited comment on MYFACES-2586 at 3/4/10 2:57 PM:
--------------------------------------------------------------

Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible I would not see this as a bug.
The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
The issue as I see here is that we have probably double ids for a fraction of a second which might collide with document.getElementById() if done inlined.





      was (Author: werpu):
    Ok Ganesh since this is your code anyway a small comment. I checked the issue, and since we do auto eval whenever possible I would not see this as a bug.
The problem lies within the auto eval, we simply add a processed range to the dom tree and at that time some browsers trigger auto eval (firefox and ie in older versions), while ie6+ and webkit need manual eval.

What happens is that at the insert stage for firefox the scripts are evaled, for ie and webkit the eval has to happen manually, if we shift that away from the old element inserted new element removed part we will have different eval behavior on older ie versions and firefox compared to the rest of the world.

Since the eval code is from you I wonder what you would suggest to fix this.
I personally would say we have to leave it as it is, since this is more or less just a cosmetic issue instead of anything else.
I rather have a similar rendering behavior over all platforms instead of a better one on some which might break things over 
other browsers.


  
> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (MYFACES-2586) wrong calling order for ajax script autorun

Posted by "Werner Punz (JIRA)" <de...@myfaces.apache.org>.
    [ https://issues.apache.org/jira/browse/MYFACES-2586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12841331#action_12841331 ] 

Werner Punz commented on MYFACES-2586:
--------------------------------------

Ok I think I have a solution, but I need a second eye to check it before commit the insert code now should be as follows to trigger the right order:
if (newTag != "") {
                var evalNode = null;
                if (typeof window.Range != 'undefined'
                        && typeof Range.prototype.createContextualFragment == 'function') {
                    var range = document.createRange();
                    range.setStartBefore(item);
                    var fragment = range.createContextualFragment(newTag);
                    evalNode = item.parentNode.replaceChild(fragment, item);
                } else {
                    item.insertAdjacentHTML('beforeBegin', newTag);
                    evalNode = item.previousSibling;
                    item.parentNode.removeChild(item);
                }

                // and remove the old item
                //first we have to save the node newly insert for easier access in our eval part
                if (myfaces._impl._util._Utils.isManualScriptEval()) {
                    myfaces._impl._util._Utils.runScripts(request, context, evalNode);
                }
                return;
            }
            // and remove the old item, in case of an empty newtag and do nothing else
            item.parentNode.removeChild(item);

We get a slightly different behavior on IE6 and ie7 due to having to revert to insertAdjacentHTML due to lack of W3C range objects. (We should rethink
the usage of innerHTML for those browsers to get the same behavior), but since neither ie6 and ie7 do any auto eval I can live with a slightly different render behavior on those browsers for now. 

Anyway please have a look at this and as soon as everyone agrees I can commit this.


> wrong calling order for ajax script autorun
> -------------------------------------------
>
>                 Key: MYFACES-2586
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2586
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.0-beta-2
>         Environment: Javascript
>            Reporter: Ganesh Jung
>
> First the new element is inserted, then the scripts are run, then the old element is removed. This is wrong, see this example:
> 	    	<h:inputText value="#{myBean.test}">
> 				<f:ajax render="test" />
> 	    	</h:inputText>
> 	    	<h:panelGroup id="test">
> 			    <script type="text/javascript">
> 					alert("running");
> 				</script>
> 		    	<h:inputText value="#{myBean.test}" />
> 	    	</h:panelGroup>
> Correct order would be: First the new element is inserted, then the old element is removed, then the scripts are run.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.