1115999 - Table cells slide offscreen

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Tom S [:evilpie]]

When moving the mouse on the page http://kangax.github.io/compat-table/es6/, table cells just slide around to the left and right. Must be recentish regression in Nightly.

Comment 1

[Tom S [:evilpie]]

Attachment: screenshot

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does it match with bug 35168 landing?

Comment 3

[Alice0775 White]

[Tracking Requested - why for this release]:

Pushlog
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=20cda36cb549&tochange=c8fd5af3c60c

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does this also happen on any other browsers?  (Probably most interested in Chrome and IE -- and maybe multiple versions of IE.)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that I'm presuming that this is happening because the page is using relative positioning and expecting it to do nothing, and we just landed a patch to make it work.

Comment 6

[Tom S [:evilpie]]

I see no problems with Chrome 41.

Comment 7

[Alice0775 White]

If open Web Console, It seems the problem does not occur....

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 35168 comment 45 and bug 35168 comment 48 say that both Chrome and IE support relative positioning moving table cells, but do not support movement of other table parts.  (I think older versions of IE did support relative positioning moving table rows, though.)

If we see compatibility problems outside of content for audiences other than the Web development and Web standards communities, I'd be more inclined to disable the feature.  But this alone isn't enough evidence to make me disable it, particularly given that I recall it working on older IE versions (e.g., from bug 845837, which I believe has content designed around relative positioning of table rows working).

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, the first sentence of the second paragraph of comment 8 had too many negatives:

I mean:  I'd be more inclined to disable the feature if we see compatibility problems in content for audiences other than the Web development and Web standards communities.

Comment 10

[Alice0775 White]

(In reply to Alice0775 White from comment #7)
> If open Web Console, It seems the problem does not occur....

s/Web Console/Inspector/

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Though I don't see anything in the CSS that's doing relative positioning on rows or rowgroups, which makes it a little more surprising that we'd get a behavior that's not present in Chrome or IE.  So maybe there's a bug in our new handling of relative positioning on cells.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, now that I see the bug, it looks like there's something broken on our side.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm suspicious of this code in nsTableRowFrame.cpp:

        // We didn't reflow. To take relative positioning into account,
        // translate the new position by the vector from the previous 'normal'
        // position to the previous position.
        // XXX(seth): This doesn't work for 'position: sticky'.
        kidPosition += kidRect.TopLeft() - origKidNormalPosition;

not updating the normal position property.  Ideally, we'd just call ApplyRelativePositioning somehow.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/5ccce7541d8f/apply-rel-pos-better fixes this; I still need to figure out how to write a test.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: mostly unminimized but self-contained testcase

Slightly simplified from original URL.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: mostly minimized testcase

The more I remove, the less reliably reproducable the bug becomes.

This is the smallest I've gotten it; I can still reproduce a decent fraction of the time by starting the mouse pointer below the first column of the table and then moving the mouse up.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Managed to build a testcase from scratch without too much trouble, though.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Call ApplyRelativePositioning properly for table cells that we choose not to reflow

I confirmed that the reftest passes with the patch and fails without the
patch.

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I should perhaps clarify;

This is the crazy codepath where nsTableRowFrame sets doReflowChild to true, and then tests some additional conditions, and based on those conditions decides to do part (but not all) of the work of reflowing the child.  This is making relative and sticky positioning work correctly in that case, which we should really just get rid of, but which I don't want to deal with now.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7dde66a57bb

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c7dde66a57bb