Closed
Bug 803677
Opened 13 years ago
Closed 13 years ago
offsetTop has wrong value for element with display:table-cell
Categories
(Core :: DOM: CSS Object Model, defect)
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: daniel, Assigned: singerb.dev)
Details
(Whiteboard: [mentor=bz])
Attachments
(2 files, 1 obsolete file)
408 bytes,
text/html
|
Details | |
3.90 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010234852
Steps to reproduce:
Set display:table-cell on an element where the parent is not table or table-row, and the previous sibling is not table-cell.
Actual results:
offsetTop is zero. This could be due to the table-cell having an *implied* container which has a non-zero offsetTop. But since offsetParent does not reference that implied container, if we try to compute the cumulative offsetTop by going through each offsetParent, we get the wrong value.
I imagine the same problem occurs with offsetLeft.
Expected results:
Either offsetTop should have a non-zero value, or offsetParent should be the implied parent of the table-cell element.
![]() |
||
Updated•13 years ago
|
Attachment #673397 -
Attachment mime type: text/plain → text/html
![]() |
||
Updated•13 years ago
|
Component: Untriaged → Layout
Product: Firefox → Core
Updated•13 years ago
|
Component: Layout → DOM: CSS Object Model
![]() |
||
Comment 1•13 years ago
|
||
Yeah, offsetTop/Left should probably skip table anonymous boxes.
If someone is interested in picking this up, I can point to the relevant code.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [mentor=bz]
Assignee | ||
Comment 2•13 years ago
|
||
Working on a patch for this. Right now I my fix deals with this bug, but then I break content/html/content/test/test_bug375003-1.html. Still, I'm hopeful I'll get it right soon.
Assignee | ||
Comment 3•13 years ago
|
||
Ignore anonymous tables when determining offsetParent (the source of offsetTop, etc) by looking at the content element as well as the frame type, and rejecting table frames that aren't from a real table.
Attachment #690681 -
Flags: review?(bzbarsky)
![]() |
||
Comment 4•13 years ago
|
||
Comment on attachment 690681 [details] [diff] [review]
Initial patch.
r=me if you move the computation of isRealTable to after we've checked the frame type, since the common case is to fail the frametype check.
Also, worth filing a followup to think about reimplementing this whole thing in terms of the element tree (which is what the spec requires and is somewhat different from our current behavior).
Attachment #690681 -
Flags: review?(bzbarsky) → review+
Comment 5•13 years ago
|
||
Benedict: I just gave your account "editbugs" permissions, so you should be able to assign bugs to yourself now (and fiddle some other bug flags).
Assignee: nobody → singerb.dev
Assignee | ||
Comment 6•13 years ago
|
||
Updated with nit from review.
Attachment #690681 -
Attachment is obsolete: true
Attachment #690852 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> Benedict: I just gave your account "editbugs" permissions, so you should be
> able to assign bugs to yourself now (and fiddle some other bug flags).
Thanks Ted!
(In reply to Boris Zbarsky (:bz) from comment #4)
> Comment on attachment 690681 [details] [diff] [review]
> Initial patch.
>
> Also, worth filing a followup to think about reimplementing this whole thing
> in terms of the element tree (which is what the spec requires and is
> somewhat different from our current behavior).
Followup filed as bug 820364.
![]() |
||
Comment 8•13 years ago
|
||
Comment on attachment 690852 [details] [diff] [review]
Final patch.
You don't need the else-after-return. So the code should look like this:
if (cell or table) {
return element_is_right_type;
}
return false;
r=me with that. No need for re-review for this change.
Attachment #690852 -
Flags: review?(bzbarsky) → review+
![]() |
||
Comment 9•13 years ago
|
||
Actually, I can just make that as I land this.
![]() |
||
Comment 10•13 years ago
|
||
Flags: in-testsuite+
Target Milestone: --- → mozilla20
Comment 11•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•