Closed Bug 803677 Opened 9 years ago Closed 9 years ago

offsetTop has wrong value for element with display:table-cell

Categories

(Core :: DOM: CSS Object Model, defect)

16 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: daniel, Assigned: singerb.dev)

Details

(Whiteboard: [mentor=bz])

Attachments

(2 files, 1 obsolete file)

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.
Attachment #673397 - Attachment mime type: text/plain → text/html
Component: Untriaged → Layout
Product: Firefox → Core
Component: Layout → DOM: CSS Object Model
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]
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.
Attached patch Initial patch. (obsolete) — Splinter Review
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 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+
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
Attached patch Final patch.Splinter Review
Updated with nit from review.
Attachment #690681 - Attachment is obsolete: true
Attachment #690852 - Flags: review?(bzbarsky)
(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 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+
Actually, I can just make that as I land this.
http://hg.mozilla.org/integration/mozilla-inbound/rev/939a0ad6e8d6
Flags: in-testsuite+
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/939a0ad6e8d6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.