783474 - Paint flex items in order of "order" property

Status: RESOLVED DUPLICATE of bug 811521

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Currently, the patches in bug 666041 have us just paint a flex container's items in frametree order.  We need to paint them in order of the "order" property, when that property is set.

I might do this as part of bug 666041, but I'm filing a bug to make sure this is addressed before we enable flexbox in builds.

(I took a crack at this before, but it depended on bug 748646 / bug 772690 -- without those bugs' patches, the display list would end up being re-sorted out from under me.

Now that those bugs are fixed, I think this should be pretty straightforward...

(Side note: Eventually we'll want to address this by re-ordering the frame tree according to "order", but we can't do that right now because tab-index and probably some a11y stuff depend on the frametree ordering, and the "order" property is explicitly _not_ supposed to affect tab-index and a11y.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Here's a testcase. The green button should be closest to the user -- it should cover part of the orange button.

The green button should also be first in the tab-index order (before the orange button).

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Here's the patch for this bug. Tests coming.

Overview of the patch:
* When we sort the children for reflow, make a note of whether mFrameList is already in sorted order.
  - If it is already sorted, then we don't need to do anything special when painting. (This keeps us from slowing down the usual-case, when "order" isn't explicitly set.)
  - If it's unsorted, then we create a sorted copy before we paint, and we use that to determine our paint-ordering.

(I also had to add a forward-declaration to let me call BuildSortedChildArray() from BuildDisplayList(), since BuildDisplayList() is towards the top of the file.)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v2

Rebased fix on top of bug 808767's patch, and added a reftest & commit message.

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 679887 [details] [diff] [review]
fix v2

Actually, per bug 811521, we're going to be sorting the actual child-frame list by "order" for other reasons -- so this bug's patch won't need to create a special sorted copy anymore.

Un-requesting review.

Comment 5

[Daniel Holbert [:dholbert]]

This may end up being trivially fixed by bug 811521, but I'm leaving it as a separate bug (dependent on bug 811521) for now.

Comment 6

[Daniel Holbert [:dholbert]]

Duping to bug 811521, since (per comment 5) this was trivially fixed by that bug and is effectively a dupe.