IE8 Script Timeout - Selection.isSelected() - Selection.cacheSelection()

[yavery]

Hi,

Once again, as part of my effort to migrate from 8.0 to 8.2p, I found another problem ... This one
much more critical as I get a script timeout under IE8. The script takes too long to execute.

I profiled my specific usage scenario with both ISC 8.0 and 8.2p and found that some of your changes
in Selection.isSelected() and Selection.cacheSelection() seem to be causing this script timeout.

I noticed that Selection.isSelected() calls Selection.cacheSelection() and Selection.cacheSelection()
also calls Selection.isSelected(). This code is probably not optimized and seem to be causing these
issues as these methods are now being called excessively in 8.2p (it wasn't the case in 8.0).

My usage scenario is pretty straight forward. I load a ListGrid from a JSON fed DataSource. 335
records get loaded with more/less 20 columns per row.

Included, are both the 8.0 and 8.2p profiling results (print screens) for my specific use case.

Any idea how I can avoid this situation or if there's something that can be changed in the ISC
framework so that this doesn't happen ?

Thanks,

[Isomorphic]

This change was related to the cascading selection feature, and none of our automated tests hit a case where you can cause some kind of loop, even though they cover lots of different variations of data loading. This suggests some kind of call order issue. Can you share details of your usage?

Note, we can blindly correct the potential for an infinite loop, but your profiling results are radically different (look at all the calls to getRawCellValue()) suggesting something more complex is going on.

[Isomorphic]

Can you confirm the exact build of 8.2p you're using? And if it's before January 24, please retest with the latest nightly build because changes have been made in this area since then.

[yavery]

I'm seeing this using 01/27 8.2p. Today I'll try a more recent build and if I still get this, I'll try to isolate
a similar scenario using only your Feature Explorer so you can see what I'm seeing, unless by doing
this exercise I figure out that it's something on our side.

However it turns out, I'll re-post later today with my findings.

Thanks,

[yavery]

Hi,

I confirm that the latest 8.2p build (02/07) has fixed the script timeout in IE8. The profiler doesn't
expose those abnormally high call rates on Selection.cacheSelection() and Selection.isSelect() anymore.

I did encounter another issue where my LinkItem form items show up with a text box instead of
an hyperlink, again ...

I'm still investigating that particular issue but do you see anything that changed between 01/27
and 02/07 that could have introduced that behavior ?

Thanks,

[yavery]

Hi,

I identified the piece of code that broke my custom LinkItem derived class ...

In FormItem (8.2p 01/27)

    // isReadOnly - helper method to determine whether a field is editable
    isReadOnly : function () {
        
        var widget = this.form;
        var fieldCanEditAttribute = (widget && widget.canEditFieldAttribute) || "canEdit";
        // Check container(s)
        var item = this;
        while (item.parentItem) {
            if (item[fieldCanEditAttribute] != null) return !item[fieldCanEditAttribute];
            item = item.parentItem;
        }
        return !isc.DynamicForm.canEditField(item, widget);
    },

In FormItem (8.2p 02/07)

    // isReadOnly - helper method to determine whether a field is editable
    // the public 'getCanEdit()' method falls through to this.
    isReadOnly : function () {
        
        var widget = this.form;
        
        // Check container(s)
        var item = this;
        while (item.parentItem) {
            if (item.canEdit != null) return !item.canEdit;
            item = item.parentItem;
        }
        return !isc.DynamicForm.canEditField(item, widget);
    },

To fix my problem, in my LinkItem derived class, I implemented the following:

	isReadOnly : function ()
	{
		return true;
	},

I'm not sure this is right though ... Can you please tell me more about this change and the
ramification your change has on my custom LinkItem derived class which renders as a text box
instead of an hyperlink, unless I implement that bogus isReadOnly() method.

Thanks,

[yavery]

I know I'm going to sound a bit pushy, but I need an answer on this quickly ... Can someone
from Isomorphic please look at this ASAP.

I need to go to production, and this is my last outstanding issue (for now ... crossing my fingers!)

Thanks,

[yavery]

I also found the following interesting situation ...

Renders my link as a text box :

DynamicForm.create
({
	ID : 'formCorpObjSearch',
	numCols : 6,
	cellPadding : 5,
	autoFocus : true,
	saveOnEnter : true,
	wrapItemTitles : false,
	titleOrientation : 'top',
	titleSuffix : '',
	dataSource : 'dsCorporateObjective',
	fields :
	[
		...,
		{
			type : 'link', name : 'switch', canEdit : false, showTitle : false, colSpan : 2,
				align : 'left', linkTitle : 'Log in as a different user', 
				click : 'MyApplication.logout(true, true)', target : 'javascript'
		},
		...
	]
});

Renders my link as an hyperlink (as expected).

DynamicForm.create
({
	ID : 'formCorpObjSearch',
	numCols : 6,
	cellPadding : 5,
	autoFocus : true,
	saveOnEnter : true,
	wrapItemTitles : false,
	titleOrientation : 'top',
	titleSuffix : '',
//	dataSource : 'dsCorporateObjective',
	fields :
	[
		...,
		{
			type : 'link', name : 'switch', canEdit : false, showTitle : false, colSpan : 2,
				align : 'left', linkTitle : 'Log in as a different user', 
				click : 'MyApplication.logout(true, true)', target : 'javascript'
		},
		...
	]
});

As you can see, the only difference is that I have a dataSource specified on my DynamicForm.

Any idea what's going on ?
Thanks,

[yavery]

I found that adding an actual field in my data source with the same name as my link field,
as specified in my DynamicForm, fixes the problem but to me that doesn't make sense ...

Lots of weird things going on here!

Thanks for your feedback,

[Isomorphic]

Hi Yan,

Firstly - yes - this is a regression and we'll have it fixed in the next nightly build (Feb 8 or greater). We were fixing another issue and missed this case.

If you're interested in the background behind the change that actually caused this:
The intended behavior for the "canEdit" attribute of a FormItem was always to set whether a field is editable or not in a DynamicForm.

However "canEdit" at the dataSource level has a slightly different meaning - it implies the data is editable. In some cases a non-editable field should still be rendered as an editable a FormItem (for example in a SearchForm) and similarly some FormItems (such as linkItems, by default) should be non-editable even if the underlying field in the dataSource is editable.

Prior to a recent change this could sometimes be difficult to achieve - you had to set (for example) "canFilter" to false on a LinkItem to have it show up as non-editable in a SearchForm.
This was non-intuitive and led to several reports on the forums which we agreed needed to be addressed.

We therefore modified behavior to match the original intent and documented everything more clearly.
The new behavior (obviously once we resolve this bug) is simply this:
- if FormItem.canEdit is specified it is respected
- otherwise if the Form is bound to a dataSource we look at the dataSource field to see if it should be editable within this form. By default we check for dataSourceField.canEdit, but for SearchForms we look at dataSoureField.canFilter. This is controlled by the dynamicForm.canEditFieldAttribute property.

Hope this clears things up and please let us know if the behavior is not resolved for you in the next nightly build
Regards
Isomorphic Software

[yavery]

Thanks,

I'll look forward to this release tomorrow. Hopefully all is good and I can move on.

I'll keep you updated.

Tx!

[yavery]

Hi,

Your fix in 8.2p 02/08, regarding the dataSource/LinkItem has fixed DynamicForm 'link'
items that are applied directly, but for some reason, whenever I have a LinkItem derived
class, such as in the following sample code, I still see those LinkItem components as
text boxes ...

isc.defineClass("AttachmentManagerItem", "LinkItem");
AttachmentManagerItem.addProperties
({
  title : "Attachments",
  linkTitle : "Attachments",
  showTitle : false,
  canEdit : false
});

isc.DataSource.create
({
ID : "dsTest",
fields :
[
  { name : "code", type : "text", length : 80 },
  { name : "name", type : "text", length : 80 }
]
});

isc.DynamicForm.create
({
  dataSource : "dsTest",
  numCols : 1,
  wrapItemTitles : false,
  useAllDataSourceFields : true,
  titleOrientation : "top",
  width : "*",
  fields:
  [
     { name : "code" },
     { name : "name" },
     { name : "X1", type : "link", showTitle : false, linkTitle : "Good Link", canEdit : false },
     { name : "X2", editorType : "AttachmentManagerItem" }
  ]
});

The only way I managed to have those LinkItem derived components work properly, is by
implementing the following code in my LinkItem derived class :

isReadOnly : function()
{
   return true;
}

I'm afraid this "patch" could break something else though ...

Any idea ? What do you suggest ?

Thanks for your quick response,

[Isomorphic]

Thanks for the clear test case (it's always invaluable to have one!).
This turned out to be an edge case we missed which we've now fixed. The fix will show up in the next nightly (Feb 9 and above)

Regards
Isomorphic Software

[yavery]

Great! I'll grab that update tomorrow AM. We'll then do a final regression testing pass on our
application using that latest build and hopefully we'll be all clear and ready to move this to
production, at last.

Thanks for your timely responses throughout this process.

Kind regards,

[yavery]

Looks good. Now, on with the full regression testing.

I'll let you know if we find anything else.

Thanks,