bug gridComplete option function call change (maybe others)

[bouks]

Consider this option.

• gridComplete: jqGridMethod.setPager

where

var jqGridMethod = {
    setPager: function() {
       console.log($(this));
    },
    otherfunction: ...
    ...
}

Until now and in all my projects the call was good and $(this) echoed the grid.

Now it doesn't work anymore. First i have to put parenthesis to enter function, like :

• gridComplete: jqGridMethod.setPager()

and then $(this) echoe the object jqGridMethod.

[flack]

I think this comes from this commit:

https://github.com/openpsa/grid.js/commit/beee9e6c2c04943dc4235750d94cbd46329e1147

The callback is now executed like this:

feedback.call(self, "gridComplete");

before, it was

ts.p.gridComplete.call(ts);

@OlegKi you might want to consider reverting the feedback logic. This is a pretty big compatibility break

[flack]

... or find a way to implement it while preserving backward compatibility. In any case, it can't stay like this IMO

[bouks]

I thought too. I tried but reverting only this don't resolve the issue.

[flack]

Could you try bisecting to see which commit is the first where it's broken?

[OlegKi]

I wrote @bouks in Skype already that the problem is not exist in my current sources. The demo http://www.ok-soft-gmbh.com/jqGrid/OK/testForGridcomplete.htm demonstrates this. I suppose two reasons: 1) you merged some preliminary version which containd a bug and I' fixed it later 2) it was some problem in merging... In any way the problem seems not exist in my current code.

you can compare the code of my current implementation of feedback https://github.com/OlegKi/jqGrid/blob/master/js/grid.base.js#L949-L972 with your.

[flack]

sorry, I think I was shortly away from my screen while that happened :-)

[OlegKi]

Yes, Laurent called my later by Skype...

[flack]

I've tried reproducing it, but for me, the pager doesn't work properly at all. It always says "Page 0 of". With local data, the next/previous buttons still work, but with remote JSON, I can only see the first page. Do you have that, too, @bouks?

[flack]

I've added a simple demo for reproducing the problem: http://openpsa.github.io/grid.js/demos/pager.html

[OlegKi]

You can look at the same demo which uses my repository. No problem. You should verify the current code of your repository...

[bouks]

It breaks at this point :

https://github.com/openpsa/grid.js/commit/d92533b4532db287883d69ad4514d8f17fbdb54d

[bouks]

@flack Same breakpoint for pager's bug

[flack]

The problem is that this i only a merge commit, i.e. it is the point at which multiple other commits were merged. I have tried bisecting it, too, and I can't find a definitive commit, because some of them produce build errors due to merge conflicts, the last candidates I get from git bisect are the following:

14a944b1b933eb352a9b703502bc6760cfec2c4b
fa9d012c2994f0296b2475c293dc30c6dfcbb1fa
ace5cb54fe3709dcd87d501a236a6aa3cdb02fc5
beee9e6c2c04943dc4235750d94cbd46329e1147
61312dc5b82c10123fcb5a4453b18ea8b94e7c8c
fc73335c563506d9196586dd5dcbab434dc805ba

So it seems to come from #60, but we have to build the individual commits manually to see from which it is exactly

[bouks]

I went to the same conclusion.

[flack]

I ran bisect again now and corrected the build failures in each iteration manually. For some, it was imposible, because there were just too many, but now bisect tells me that the problem comes from here: https://github.com/openpsa/grid.js/commit/fc73335c563506d9196586dd5dcbab434dc805ba

I don't have time to look into this right away, but it might be a good starting point to do some checking. I currently suspect that it has something to do with getGridComponent, because the function is buggy in this particular commit, but more testing is required

[flack]

One more thing I found out: updatepager is not called in our version, but it is in Oleg's

[flack]

Should be fixed now. The problem was that the number of parameters for addJSONData has changed from 5 to 4. So it was an API change by @OlegKi after all, namely this commit: https://github.com/OlegKi/jqGrid/commit/868c9c556b83b8b6731ad21bacdfb34f3c13783b

We have the same problem with addXmlData btw, I'll add another demo & fix in a little while

[bouks]

Thanks @flack