#1965 closed defect (fixed)
[patch] [cla] getAbsolutePosition() broken for scrolled elements in Mozilla
Reported by: | Owned by: | sjmiles | |
---|---|---|---|
Priority: | high | Milestone: | 0.9 |
Component: | HTML | Version: | 0.4 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
There is a bug in
dojo.html.getAbsolutePosition()
when run on Mozilla.
What happens is that the reported position is affected by the node's internal scroll, so if I am asking about a scrollable DIV's position I get different values depending on how the DIV's content is scrolled. This doesn't happen on IE.
I traced the problem down to sumAncestorProperties() that is only used for Mozilla, and the bug is that this function is not only summing values from ancestors but also from the node itself. With the two added lines below it works correctly:
dojo.html.sumAncestorProperties = function(/* HTMLElement */node, /* string */prop){ // summary // Returns the sum of the passed property on all ancestors of node. node = dojo.byId(node); if(!node){ return 0; } // FIXME: throw an error? node = node.parentNode; // *** ADD THIS if(!node){ return 0; } // *** ADD THIS var retVal = 0; while(node){ ...
Best regards Mike
Change History (12)
comment:1 Changed 14 years ago by
comment:3 Changed 14 years ago by
Summary: | getAbsolutePosition() broken for scrolled elements in Mozilla → [patch] [cla] getAbsolutePosition() broken for scrolled elements in Mozilla |
---|
CLA confirmed.
comment:4 Changed 14 years ago by
#1947 is not affected by the fix, sorry.
dojobug[at]cpusoftware.com
comment:5 Changed 14 years ago by
comment:6 Changed 14 years ago by
Hi there,
Sorry for nagging, but why isn't anything happening with this bug? It is an easy bug to verify, and the solution is simple (and included in the report).
I'll provide a more detailed explanation below. Please let me know if there is anything more you want me to clarify.
Consider the following element tree:
DIV a (height=100) DIV b (scrollTop=20) c
(The page first has one DIV that currently is 100px high, followed by a DIV with one child that is scrolled 20px)
I want to know the top position of "b" (should be 100) and call dojo.html.getAbsolutePosition(b). In Mozilla, it will do something like:
box = document.getBoxObjectFor(b); height = box.y - dojo.html.sumAncestorProperties(node, "scrollTop");
box.y will be 100, but the call to sumAncestorProperties will return 20, thereby making the calculation return 80. The reason is because sumAncestorProperties includes element b's own scrollTop in its sum, which of course isn't relevant to b's position (it has the same position independent of how it has scrolled its own children). The function sumAncestorProperties's name suggests that it is only ancestors' properties that should be summed, and not properties from the node itself, and this is not the case with the current implementation.
I understand that you maybe are planning a major refactoring of this function (maybe due to https://bugzilla.mozilla.org/show_bug.cgi?id=340571) and have not had time to do this yet, but couldn't you check in my suggested fix in the meantime?
Best regards Mike
comment:7 Changed 14 years ago by
Yeah, I guess that according to the comment for function sumAncestorProperties():
// summary // Returns the sum of the passed property on all ancestors of node.
is shouldn't include the node itself in the calculations (just it's ancestors), so your patch seems to make sense.
comment:8 Changed 14 years ago by
Milestone: | → 0.9 |
---|
comment:9 Changed 14 years ago by
Owner: | changed from Bryan Forbes to sjmiles |
---|
comment:10 Changed 14 years ago by
Status: | new → assigned |
---|
Sorry this languished for so long. I only became owner of this recently as you can see, and our review shows your report and fix to be spot on.
comment:11 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This solves #1621. At least for Firefox. Opera still has issues.