jQuery Grid Plugin - jqGrid

Good catch... I'd stared at those lines, and still not seen it.

But that reminds me - why not have a $.jgrid.getCellIndex(cell), which does both steps for you (ie cell.cellIndex, and then getAbsoluteIndex when necessary)? There's a lot of places where thats done, and it would make things less error prone.

Also, wouldnt $.inArray(cell, cell.parentNode.cells) be a simple/efficient enough implementation? Then no need for the browser sniffing/fixup?

Let me know if you find any more fixes - I'll take a look at the rest tonight...

Mark

Tony,

I've not managed to reproduce your tree grid issue, but I do see a possibly releated problem...

Depending on the page, ie8 will sometimes get cellIndex right (like FF etc), and sometimes get it wrong (like ie6,7). So your test in getAbsoluteIndex for browser.version <= 7 doesnt really work.

The problem is that when ie8 runs in compatibility mode, it still reports its version number as 8, but emulates the behavior of 7.

The only fix I can think of is what I suggested above, so I've added $.jgrid.getCellIndex (which uses inArray for msie, and cellIndex for everything else), and removed getAbsoluteIndex altogether.

I've pushed the changes to my fork - perhaps this will fix your problems too?

On the sortable bug - I think it /is/ a bug in sortable, but I dont think its that one. That happens when you're trying to sort rows, because (apparently) webkit doesnt render empty <tr> elements, and so the placeholder doesnt show up. But in this case, its an empty <th> element. And in any case, it /does/ show up. Its the cell in the row below that disappears!

Mark

So it's actually an interesting bug - probably in ie8 - although I think its more just a case of ie8 doing something surprising.

jQuery makes the assumption that anything hidden will have zero size. But hidden td/th elements in ie8 actually have the size of the tr element that contains them.

So this breaks two things. First the ":hidden" selector doesnt work for td/th elements, and so my "items" selector selects all the columns (hidden or not) for sorting.

That wouldnt matter, except that sortable doesnt test to see if the items are visible, it just assumes that hidden items will have zero size, and so does no special checking. As a result, the mouse is /always/ over the hidden items.

Fortunately, the fix is simple. For $.browser.msie && $.browser.version==8, I redefined the ":hidden" attribute to additionally check for style.display=="none". So the hidden items dont make it into the sortable list, and everything works.

Ill push to gitHub in a while.

Mark

Hi Tony,

Great - and thanks for all your work!

Putting the sortable code in a separate module certainly makes sense (although it doesnt /depend/ on jquery-ui - it simply doesnt do anything unless sortable is loaded). But remapColumns is self standing. My code uses it in a number of places independently of the sortable code (I added sortable support as an afterthought).

It doesnt really matter to me - because I will be including the sortable code anyway - but it seems like remapColumns should be a part of the base functionality.

Mark

Thanks.

I also just noticed you took all the newapi changes - thats great! My fork matches yours 🙂

Would it make sense to make a 3.5.3 branch (or maybe call it 3.5, but based off 3.5.3)? Particularly after the newapi changes, it may make sense to push bugfixes to 3.5 while working on 3.6...

Mark