Issue with getPosition about tablednd HOT 13 CLOSED

Included in release v1.0.4, thanks both

from tablednd.

You'd need to provide your table code to look at :)

from tablednd.

Sure, I created this jsfiddle.
https://jsfiddle.net/pirobox/hkymq2dp/2/

thanks in advance.

from tablednd.

That's an interesting one. I might have to defer to @isocra on this one. Removing the row with the blank cells makes it work, as you say.

Is element.firstChild trying to find the first td or the first child of the td? Which in an empty node would be nothing, but in a node with text in it would be the textNode?

from tablednd.

ok. Locally I have solved the issue checking whether the element is null or not before trying to access its properties

getPosition: function(element) {
    var left = 0,
        top  = 0;

    // Safari fix -- thanks to Luis Chato for this!
    // Safari 2 doesn't correctly grab the offsetTop of a table row
    // this is detailed here:
    // http://jacob.peargrove.com/blog/2006/technical/table-row-offsettop-bug-in-safari/
    // the solution is likewise noted there, grab the offset of a table cell in the row - the firstChild.
    // note that firefox will return a text node as a first child, so designing a more thorough
    // solution may need to take that into account, for now this seems to work in firefox, safari, ie
    if (element.offsetHeight === 0)
        element = element.firstChild; // a table cell

    while (element && element.offsetParent) {
        left   += element.offsetLeft;
        top    += element.offsetTop;
        element = element.offsetParent;
    }

	if(element){
		left += element.offsetLeft;
		top  += element.offsetTop;
	}

    return {
        x: left,
        y: top
    };
}

Interesting is that this issue is the same of this
#87
Infact the html of the table I provided is the one generated by the jqgrid.
So solving this issue would actually close two tickets :)

regards,

from tablednd.

We probably don't need that Safari fix in there anymore! :) Unless current safari still has that problem.

Have you tried without:

if (element.offsetHeight === 0)
        element = element.firstChild; // a table cell

completely? It looks like the bug fix for Safari was to grab the child of the td to make calculations off.

from tablednd.

No I didn't try because I have no expertise with Safari and I wouldn't mess with some hack for it (if I can avoid it).

regards,

from tablednd.

Give it a go, perhaps in a PR, and if it solves your problem I'll give it a whirl in current Safari. If it works, then I think we can safely drop that hack and just do a minor release with a breaking change notice.

from tablednd.

it works in my local environment.
let's say that if we are not sure about the scenario that required that hack for safari it is safer to use my approach and testing whether the element is null or not before using it.
In any case, I'm not the owner of the software, so for me any solution that solves my bug is ok.

regards,

from tablednd.

I'm not the owner either but I can merge PR's from memory. I think it's safe to look at removing this hack, so if you're comfortable, go ahead and create a PR that removes this hack and I'll test it out before merging.

from tablednd.

Hi Brendon,
unfortunately I have no idea what PR is.
Moreover I have already fixed the issue in my web application, so, I'm only signaling the issue for someone to fix in the git repository.

regards,

from tablednd.

By the end, I created this pull request.
Hope it is useful

#103

from tablednd.

Hi @pirobox, well done on your first (I presume?) Pull Request! :D Thank you for doing this as it makes our job a bit easier as maintainers. I've tested the change and it works well on modern Safari so I don't think we need that hack anymore. I've asked @isocra to chime in just in case he has any background information on it.

from tablednd.