Closed Bug 850684 Opened 12 years ago Closed 7 years ago

offsetLeft / offsetTop get wrong value when offsetParent 's border is set, parent has non-visible overflow, and child is absolutely positioned

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: yuqingcai, Assigned: bzbarsky)

Details

(Keywords: testcase, Whiteboard: [bugday-20131002])

Attachments

(4 files)

testcase put together from comment #0
1.63 KB, text/html
Details
testcase put together from comment #0, no border property in style1
1.60 KB, text/html
Details
Minimal-ish testcase
255 bytes, text/html
Details
Make sure offset* is computed correctly on absolutely positioned kids of relatively positioned elements with scrolllframes and borders
13.79 KB, patch
MatsPalmgren_bugz
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.17 (KHTML, like Gecko) Ubuntu Chromium/24.0.1312.56 Chrome/24.0.1312.56 Safari/537.17 Steps to reproduce: I created two div element,div1 contain div2, css source: div.style1{ margin: 10px; border: 10px solid #FF0000; padding: 10px; left: 10px; top: 10px; width: 300px; height: 300px; background-color: #FFFFFF; position: absolute; overflow: hidden; } div.style2{ margin: 10px; border: 10px solid #00FF00; padding: 10px; left: 10px; top: 10px; width: 200px; height: 200px; background-color: #FFFFFF; position: absolute; overflow: hidden; } html source: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head id="head"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> <title> find parent </title> <link rel="stylesheet" type="text/css" href="/style/common.css" /> <script src="/js/find_parents.js"></script> </head> <body> <div id="div1" class="style1"> <div id="div2" class="style2" onmousedown="showInfo(event, this)"> test... </div> </div> <div id="info" class="infoStyle"> </div> </body> </html> js source: function getObjOffset(obj, offset) { if (!obj) { return ; } offset.x += obj.offsetLeft + obj.clientLeft; offset.y += obj.offsetTop + obj.clientTop; getObjOffset(obj.offsetParent, offset); } function getObjPos(obj, offset) { getObjOffset(obj, offset); offset.x -= obj.clientLeft; offset.y -= obj.clientTop; } function showInfo(evn, obj) { var offset = {x: 0, y: 0}; getObjPos(obj, offset); var infoObj = document.getElementById("info"); infoObj.innerHTML = offset.x + ", " + offset.y + "<br/>" + evn.clientX + ", " + evn.clientY; } Actual results: when I run the javascript code, I noticed that the element of div2's offsetLeft and offsetTop get a wrong value, the offsetLeft is 10px, and the offsetTop is 10px。 Expected results: the div2's offsetLeft /offsetTop should be 20px, it equal div1's margin 10px + div1's left 10px / div1's top 10px. I use the Chrome, IE and safari to confirm it. When I remove the div1's border Property, the rusult is correct.
Could you add a complete working testcase as an attachment please?
Keywords: testcase
The testcases currently work inconveniently: the numbers appear behind/under the divs.
Component: Untriaged → DOM
Product: Firefox → Core
Whiteboard: [bugday-20131002]
Attached file Minimal-ish testcase —
This happens when an abs-pos element has a containing block with a scrollframe. In nsGenericHTMLElement::GetOffsetRect we get true for isAbsolutelyPositioned so don't accumulate offsets going up the tree, but that means that our offsetLeft just ends up being our offset from the _scrolled_ frame (which is the frame tree parent in this case), not from the _scrollframe_. And the offset between the two of them is precisely the border-width of the parent. Why exactly do we skip the position accumulation for the isAbsolutelyPositioned case?
Flags: needinfo?(matspal)
Summary: offsetLeft / offsetTop get wrong value when offsetParent 's border is set → offsetLeft / offsetTop get wrong value when offsetParent 's border is set, parent has non-visible overflow, and child is absolutely positioned
Status: UNCONFIRMED → NEW
Ever confirmed: true
I don't know, I only added the !isOffsetParent check there. The !isAbsolutelyPositioned part dates back to: 1.264 <jst@netscape.com> 2001-06-29 15:44 Fixing bug 81290. The element.offsetXXX properties contained incorrect values when the element is positioned, or a child of a positioned element, this made these properties kinda useless since we were nowhere close to IE wrt the values of these properties. r=pollmann@netscape.com, sr=vidur@netscape.com http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsGenericHTMLElement.cpp&rev=1.264&root=/cvsroot#843 I get the same result for your test in FF3.6 for example. Perhaps this was the "correct" result in IE at the time?
Flags: needinfo?(matspal)
Hard to say. Sounds like we should do some testing in the various other edge cases here, unless the spec editor has already done that...
Flags: needinfo?(annevk)
Flags: needinfo?(annevk)
Anne, does removing the needinfo mean that the edge cases have in fact already been tested by someone and what's in the spec matches most implementations?
Flags: needinfo?(annevk)
Sorry, I removed myself because Simon is the editor these days. I have done testing http://dump.testsuite.org/2006/dom/style/offset/ but the tests are old and browsers have been tweaked I suppose. I had some tool too but I cannot find it.
Flags: needinfo?(annevk) → needinfo?(zcorpan)
OS: Linux → All
Hardware: x86_64 → All
Version: 19 Branch → Trunk
Both Chrome and IE return '0' in Boris' testcase. Whether this is correct or not, I think it should be fixed, if just for consistency between current browsers.
OK, so I dug through bug 81290 and related bits a bit. It looks to me like the fix for bug 81290 was just wrong. It should have been fixed similar to how bug 375003 was fixed, but not adjusting for the position of the offsetParent itself. So I think we can in fact just remove that isAbsolutelyPositioned bit. With it removed, we still pass the testcase from bug 81290, now that bug 375003 is fixed. Try push with that change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6725632ce1559c1eb8386069278f4e6093b336af
The check for isAbsolutelyPositioned was an old incorrect attempt to fix bug 81290. We have since added the proper fix (not adding offsets of the offset parent frame) in bug 375003. MozReview-Commit-ID: 7NgnfrYcs8h
Attachment #8912008 - Flags: review?(mats)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
I filed https://github.com/w3c/csswg-drafts/issues/1832 on the spec here being totally bogus.
Comment on attachment 8912008 [details] [diff] [review] Make sure offset* is computed correctly on absolutely positioned kids of relatively positioned elements with scrolllframes and borders LGTM. r=mats
Attachment #8912008 - Flags: review?(mats) → review+
FYI, <colgroup>, <col> and <option>.offsetTop are incompatible with Chrome. I've filed bug 1403014.
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8377625a20c1 Make sure offset* is computed correctly on absolutely positioned kids of relatively positioned elements with scrolllframes and borders. r=mats
https://hg.mozilla.org/mozilla-central/rev/8377625a20c1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Flags: needinfo?(zcorpan) → needinfo?(emilio)
Let's track this in https://github.com/w3c/csswg-drafts/issues/1832. I filed https://github.com/w3c/csswg-drafts/issues/3019 for bug 1403014. Boris, you're awesome, thanks for the tests from this bug :)
Flags: needinfo?(emilio)
You're welcome!
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: