[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
Looked a bit deeper into this issue. Seems that the suggested change to MochiKit.DOM.getElement() would break some tests for Signal and DOM, since both modules contain code that assume getElement() will just return whatever non-string object it was provided with. I've fixed those cases I could find (r1492), but I guess this might be an API-breaking change. So I'm holding off on the core MochiKit.DOM.getElement() change for now. Regarding the original issue, I think that the original code attempted to say this (not perfect, but more understandable): getElementPosition: function (elem, /* optional */relativeTo) { var self = MochiKit.Style; var dom = MochiKit.DOM; var isCoordinates = function (o) { return o != null && o.nodeType == null && typeof(o.x) == "number" && typeof(o.y) == "number"; } if (typeof(elem) == "string") { elem = dom.getElement(elem); } if (elem == null || (!isCoordinates(elem) && self.getStyle(elem, 'display') == 'none')) { return undefined; } Fixed in r1494. Thanks for the feedback! Cheers, /Per On Mon, Dec 15, 2008 at 1:11 PM, Per Cederberg wrote: > Ah, yes... You are probably right. Thanks for the analysis! > > It only makes sense for getElement() to return a Node or > null/undefined. Other return values are probably just a side-effect > for the fast-path return for DOM nodes. > > But perhaps fixing this will break one or two things, like the > getElementPosition() code mentioned... I'll check that out once I'm > done with some other MochiKit work I'm currently doing. > > I still suspect the weird if-statement posted before for not being sane, > though. > > Cheers, > > /Per > > On Mon, Dec 15, 2008 at 12:59 PM, Amit Mendapara > wrote: >> >> I think, >> >> var elem = MochiKit.DOM.getElement({}); >> >> should return `null` or `undefined` but it returns the argument in >> this case. The check >> >> if (!elem || elem == d) { >>return undefined; >> } >> >> in getStyle fails due to this... >> >> - Amit >> >> On Dec 15, 4:34 pm, "Per Cederberg" wrote: >>> Naturally I meant: >>> >>> getElementPosition(descendant, parent); >>> >>> ... and not the other way around. >>> >>> /Per >>> >>> On Mon, Dec 15, 2008 at 12:34 PM, Per Cederberg wrote: >>> > I think that line was used to do the following: >>> >>> >var parent = $("one"); >>> >var descendant = $("two"); >>> >getElementPosition(parent, descendant); >>> >>> > I.e, we can send another node as the relativeTo value. Not just an >>> > object with x and y properties. >>> >>> > Cheers, >>> >>> > /Per >>> >>> > On Mon, Dec 15, 2008 at 12:17 PM, Amit Mendapara >>> > wrote: >>> >>> >> What does this line really intended for? >>> >>> >> relativeTo = arguments.callee(relativeTo); >>> >>> >> I have removed this line and that error was gone... >>> >>> >> - Amit >>> >>> >> On Dec 12, 8:08 pm, "Per Cederberg" wrote: >>> >>> Hi, >>> >>> >>> I ran across a weird bug in MochiKit.Style.getElementPosition causing >>> >>> FF to throw evil C++ exceptions into the console: >>> >>> >>>http://trac.mochikit.com/ticket/332 >>> >>> >>> Debugging the MochiKit code I ended up looking at the following piece >>> >>> of black magic: >>> >>> >>> getElementPosition: function (elem, /* optional */relativeTo) { >>> >>> var self = MochiKit.Style; >>> >>> var dom = MochiKit.DOM; >>> >>> elem = dom.getElement(elem); >>> >>> >>> if (!elem || >>> >>> (!(elem.x && elem.y) && >>> >>> (!elem.parentNode === null || >>> >>> self.getStyle(elem, 'display') == 'none'))) { >>> >>> return undefined; >>> >>> } >>> >>> >>> Question: What does the if-statement really do? And what was the real >>> >>> intention? >>> >>> >>> It seems the getStyle() function is called even though I send in a { >>> >>> x: 0, y: 0 } object. I guess that is not the real intention. >>> >>> Especially I like the "!elem.parentNode === null" check. What does >>> >>> that even mean??? Weird that the previous test cases haven't caught >>> >>> anything here... >>> >>> >>> Cheers, >>> >>> >>> /Per >> >> >> > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
Ah, yes... You are probably right. Thanks for the analysis! It only makes sense for getElement() to return a Node or null/undefined. Other return values are probably just a side-effect for the fast-path return for DOM nodes. But perhaps fixing this will break one or two things, like the getElementPosition() code mentioned... I'll check that out once I'm done with some other MochiKit work I'm currently doing. I still suspect the weird if-statement posted before for not being sane, though. Cheers, /Per On Mon, Dec 15, 2008 at 12:59 PM, Amit Mendapara wrote: > > I think, > > var elem = MochiKit.DOM.getElement({}); > > should return `null` or `undefined` but it returns the argument in > this case. The check > > if (!elem || elem == d) { >return undefined; > } > > in getStyle fails due to this... > > - Amit > > On Dec 15, 4:34 pm, "Per Cederberg" wrote: >> Naturally I meant: >> >> getElementPosition(descendant, parent); >> >> ... and not the other way around. >> >> /Per >> >> On Mon, Dec 15, 2008 at 12:34 PM, Per Cederberg wrote: >> > I think that line was used to do the following: >> >> >var parent = $("one"); >> >var descendant = $("two"); >> >getElementPosition(parent, descendant); >> >> > I.e, we can send another node as the relativeTo value. Not just an >> > object with x and y properties. >> >> > Cheers, >> >> > /Per >> >> > On Mon, Dec 15, 2008 at 12:17 PM, Amit Mendapara >> > wrote: >> >> >> What does this line really intended for? >> >> >> relativeTo = arguments.callee(relativeTo); >> >> >> I have removed this line and that error was gone... >> >> >> - Amit >> >> >> On Dec 12, 8:08 pm, "Per Cederberg" wrote: >> >>> Hi, >> >> >>> I ran across a weird bug in MochiKit.Style.getElementPosition causing >> >>> FF to throw evil C++ exceptions into the console: >> >> >>>http://trac.mochikit.com/ticket/332 >> >> >>> Debugging the MochiKit code I ended up looking at the following piece >> >>> of black magic: >> >> >>> getElementPosition: function (elem, /* optional */relativeTo) { >> >>> var self = MochiKit.Style; >> >>> var dom = MochiKit.DOM; >> >>> elem = dom.getElement(elem); >> >> >>> if (!elem || >> >>> (!(elem.x && elem.y) && >> >>> (!elem.parentNode === null || >> >>> self.getStyle(elem, 'display') == 'none'))) { >> >>> return undefined; >> >>> } >> >> >>> Question: What does the if-statement really do? And what was the real >> >>> intention? >> >> >>> It seems the getStyle() function is called even though I send in a { >> >>> x: 0, y: 0 } object. I guess that is not the real intention. >> >>> Especially I like the "!elem.parentNode === null" check. What does >> >>> that even mean??? Weird that the previous test cases haven't caught >> >>> anything here... >> >> >>> Cheers, >> >> >>> /Per > > > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
I think, var elem = MochiKit.DOM.getElement({}); should return `null` or `undefined` but it returns the argument in this case. The check if (!elem || elem == d) { return undefined; } in getStyle fails due to this... - Amit On Dec 15, 4:34 pm, "Per Cederberg" wrote: > Naturally I meant: > > getElementPosition(descendant, parent); > > ... and not the other way around. > > /Per > > On Mon, Dec 15, 2008 at 12:34 PM, Per Cederberg wrote: > > I think that line was used to do the following: > > > var parent = $("one"); > > var descendant = $("two"); > > getElementPosition(parent, descendant); > > > I.e, we can send another node as the relativeTo value. Not just an > > object with x and y properties. > > > Cheers, > > > /Per > > > On Mon, Dec 15, 2008 at 12:17 PM, Amit Mendapara > > wrote: > > >> What does this line really intended for? > > >> relativeTo = arguments.callee(relativeTo); > > >> I have removed this line and that error was gone... > > >> - Amit > > >> On Dec 12, 8:08 pm, "Per Cederberg" wrote: > >>> Hi, > > >>> I ran across a weird bug in MochiKit.Style.getElementPosition causing > >>> FF to throw evil C++ exceptions into the console: > > >>> http://trac.mochikit.com/ticket/332 > > >>> Debugging the MochiKit code I ended up looking at the following piece > >>> of black magic: > > >>> getElementPosition: function (elem, /* optional */relativeTo) { > >>> var self = MochiKit.Style; > >>> var dom = MochiKit.DOM; > >>> elem = dom.getElement(elem); > > >>> if (!elem || > >>> (!(elem.x && elem.y) && > >>> (!elem.parentNode === null || > >>> self.getStyle(elem, 'display') == 'none'))) { > >>> return undefined; > >>> } > > >>> Question: What does the if-statement really do? And what was the real > >>> intention? > > >>> It seems the getStyle() function is called even though I send in a { > >>> x: 0, y: 0 } object. I guess that is not the real intention. > >>> Especially I like the "!elem.parentNode === null" check. What does > >>> that even mean??? Weird that the previous test cases haven't caught > >>> anything here... > > >>> Cheers, > > >>> /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
Naturally I meant: getElementPosition(descendant, parent); ... and not the other way around. /Per On Mon, Dec 15, 2008 at 12:34 PM, Per Cederberg wrote: > I think that line was used to do the following: > >var parent = $("one"); >var descendant = $("two"); >getElementPosition(parent, descendant); > > I.e, we can send another node as the relativeTo value. Not just an > object with x and y properties. > > Cheers, > > /Per > > On Mon, Dec 15, 2008 at 12:17 PM, Amit Mendapara > wrote: >> >> What does this line really intended for? >> >> relativeTo = arguments.callee(relativeTo); >> >> I have removed this line and that error was gone... >> >> - Amit >> >> On Dec 12, 8:08 pm, "Per Cederberg" wrote: >>> Hi, >>> >>> I ran across a weird bug in MochiKit.Style.getElementPosition causing >>> FF to throw evil C++ exceptions into the console: >>> >>>http://trac.mochikit.com/ticket/332 >>> >>> Debugging the MochiKit code I ended up looking at the following piece >>> of black magic: >>> >>> getElementPosition: function (elem, /* optional */relativeTo) { >>> var self = MochiKit.Style; >>> var dom = MochiKit.DOM; >>> elem = dom.getElement(elem); >>> >>> if (!elem || >>> (!(elem.x && elem.y) && >>> (!elem.parentNode === null || >>> self.getStyle(elem, 'display') == 'none'))) { >>> return undefined; >>> } >>> >>> Question: What does the if-statement really do? And what was the real >>> intention? >>> >>> It seems the getStyle() function is called even though I send in a { >>> x: 0, y: 0 } object. I guess that is not the real intention. >>> Especially I like the "!elem.parentNode === null" check. What does >>> that even mean??? Weird that the previous test cases haven't caught >>> anything here... >>> >>> Cheers, >>> >>> /Per >> >> >> > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
I think that line was used to do the following: var parent = $("one"); var descendant = $("two"); getElementPosition(parent, descendant); I.e, we can send another node as the relativeTo value. Not just an object with x and y properties. Cheers, /Per On Mon, Dec 15, 2008 at 12:17 PM, Amit Mendapara wrote: > > What does this line really intended for? > > relativeTo = arguments.callee(relativeTo); > > I have removed this line and that error was gone... > > - Amit > > On Dec 12, 8:08 pm, "Per Cederberg" wrote: >> Hi, >> >> I ran across a weird bug in MochiKit.Style.getElementPosition causing >> FF to throw evil C++ exceptions into the console: >> >>http://trac.mochikit.com/ticket/332 >> >> Debugging the MochiKit code I ended up looking at the following piece >> of black magic: >> >> getElementPosition: function (elem, /* optional */relativeTo) { >> var self = MochiKit.Style; >> var dom = MochiKit.DOM; >> elem = dom.getElement(elem); >> >> if (!elem || >> (!(elem.x && elem.y) && >> (!elem.parentNode === null || >> self.getStyle(elem, 'display') == 'none'))) { >> return undefined; >> } >> >> Question: What does the if-statement really do? And what was the real >> intention? >> >> It seems the getStyle() function is called even though I send in a { >> x: 0, y: 0 } object. I guess that is not the real intention. >> Especially I like the "!elem.parentNode === null" check. What does >> that even mean??? Weird that the previous test cases haven't caught >> anything here... >> >> Cheers, >> >> /Per > > > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---
[mochikit] Re: Weird bug in MochiKit.Style.getElementPosition
What does this line really intended for? relativeTo = arguments.callee(relativeTo); I have removed this line and that error was gone... - Amit On Dec 12, 8:08 pm, "Per Cederberg" wrote: > Hi, > > I ran across a weird bug in MochiKit.Style.getElementPosition causing > FF to throw evil C++ exceptions into the console: > > http://trac.mochikit.com/ticket/332 > > Debugging the MochiKit code I ended up looking at the following piece > of black magic: > > getElementPosition: function (elem, /* optional */relativeTo) { > var self = MochiKit.Style; > var dom = MochiKit.DOM; > elem = dom.getElement(elem); > > if (!elem || > (!(elem.x && elem.y) && > (!elem.parentNode === null || > self.getStyle(elem, 'display') == 'none'))) { > return undefined; > } > > Question: What does the if-statement really do? And what was the real > intention? > > It seems the getStyle() function is called even though I send in a { > x: 0, y: 0 } object. I guess that is not the real intention. > Especially I like the "!elem.parentNode === null" check. What does > that even mean??? Weird that the previous test cases haven't caught > anything here... > > Cheers, > > /Per --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "MochiKit" group. To post to this group, send email to mochikit@googlegroups.com To unsubscribe from this group, send email to mochikit+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/mochikit?hl=en -~--~~~~--~~--~--~---