RESOLVED FIXED 19094
offsetTop is wrong in cell <td> 
https://bugs.webkit.org/show_bug.cgi?id=19094
Summary offsetTop is wrong in cell <td>
Jean-Sébastien Goupil
Reported 2008-05-15 17:08:24 PDT
When you have a cell that specifies its height, the offsetTop will be based on where the text will be written. It should be based where the TD starts. The workaround is to set the vertical-align: top to the TD. We tried with the latest version of Safari3.1.1(527.17) on Windows2003 and the night build on MAC. This is the HTML <html> <head> <script type="text/javascript"> function loaded() { alert("Offset of the TD: " + document.getElementById("thetd").offsetLeft + "X" + document.getElementById("thetd").offsetTop); } </script> </head> <body onload="loaded()" id="body"> <table style="background-color: orange;"><tr style="background-color: blue;"><td id="thetd" style="/*vertical-align:top;*/width: 100px; height: 500px;background-color: red"></td></tr></table> </body> </html>
Attachments
patch that corrects offsetTop for table cells (603 bytes, patch)
2008-08-02 15:59 PDT, Bernhard Kruepl 		no flags
DetailsFormatted DiffDiff
patch that corrects offsetTop for table cells with test (6.82 KB, patch)
2008-08-03 05:58 PDT, Bernhard Kruepl 		hyatt: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Bernhard Kruepl
Comment 1 2008-08-02 15:59:12 PDT
Created attachment 22620 [details] patch that corrects offsetTop for table cells
Bernhard Kruepl
Comment 2 2008-08-02 16:12:51 PDT
This bug hit me when I wanted to extract table cell coordinates (latest SVN, OSX 10.5.4). Quite strangely, offsetTop lies intentionally: RenderTableCell.h, line100: // Lie about position to outside observers. virtual int yPos() const { return m_y + m_topExtra; } My patch corrects the lie by subtracting the borderTopExtra again. It breaks the regression, fast/dom/Element/offsetTop-table-cell.html explicitly asks for wrong values. I think this test is just wrong and should also be patched.
Bernhard Kruepl
Comment 3 2008-08-03 05:58:12 PDT
Created attachment 22623 [details] patch that corrects offsetTop for table cells with test I created a patch for offsetTop, updated and extended the appropriate test. I also created a changelog entry. Hope this meets all requirements, the bug seems to have been present for eight years now...
Eric Seidel (no email)
Comment 4 2008-08-04 01:36:47 PDT
Beth is the right person to review this.
Beth Dakin
Comment 5 2008-08-04 12:36:15 PDT
I have a feeling that there is a reason offsetTop lied intentionally. I don't know why it was coded that way originally though. I am cc-ing Hyatt because he might have more information about that. But I am very worried about this change, especially considering the test and the comment.
Dave Hyatt
Comment 6 2008-08-04 12:44:44 PDT
Comment on attachment 22623 [details] patch that corrects offsetTop for table cells with test This looks good actually. r=me.
Dave Hyatt
Comment 7 2008-08-04 12:45:08 PDT
offsetTop is lying unintentionally because it used yPos().
mitz
Comment 8 2008-08-04 13:27:39 PDT
The isTableCell() check is an unnecessary virtual method call, because borderTopExtra() is 0 for anything else. If I land this patch, I will just remove it.
Bernhard Kruepl
Comment 9 2008-08-04 16:31:49 PDT
Fair enough. And you are right, lying was yPos()'s intention, not offsetTop's.
mitz
Comment 10 2008-08-04 21:10:26 PDT
Landed in <http://trac.webkit.org/changeset/35551>.
Note You need to log in before you can comment on or make changes to this bug.