1148294 - Take 'writing-mode' into consideration when setting up FlexboxAxisTracker

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Right now, FlexboxAxisTracker takes "direction" (ltr or rtl) into consideration when setting up the flex axes. But it doesn't take "writing-mode" into consideration.

It should.

Comment 1

[Daniel Holbert [:dholbert]]

The spec text for this, FWIW, is:
 http://dev.w3.org/csswg/css-flexbox-1/#propdef-flex-direction

e.g. for flex-direction:row:
 # row
 #   The flex container’s main axis has the same
 #   orientation as the inline axis of the current
 #   writing mode. The main-start and main-end
 #   directions are equivalent to the inline-start
 #   and inline-end directions, respectively, of the
 #   current writing mode. 
and so on for row-reverse, column, column-reverse, etc.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: the fix

This patch does several things:
 (1) It makes the FlexboxAxisTracker constructor take a nsStylePosition and a WritingMode (the two key things we now need to determine the axes), instead of a nsIFrame*. That way, we can use the cached nsStylePosition & WritingMode on the reflow state, during reflow.

 (2) It adds convenience functions for converting WritingMode::InlineDir and WritingMode::BlockDir into AxisOrientationType (the enum that we currently use to represent the flex axes).  These will likely go away soon (since we probably want to convert AxisOrientationType into something more logical, as discussed in bug 1148298) -- but for now, this is an incremental step towards actually using the WritingMode.

 (3) It makes us query the passed-in WritingMode to get our inline axis & block axis (instead of simply assuming that block is top-to-bottom and inline is LTR or RTL depending on 'direction).

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 1 v2: the fix

(Ah, I also just noticed we don't need the 'explicit' keyword on the constructor anymore, since it's becoming a multi-arg constructor. Here's an updated patch that also removes the 'explicit'.)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 2: first reftest for 'writing-mode' & 'direction' influencing flex axes

Here's a first reftest for this behavior. More reftests coming up in a second patch, using 'hg cp' to borrow liberally from this one. (just changing 'direction' & 'writing-mode')

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: part 3: more reftests (using 'hg cp')

...and here are the rest of the reftests.

The two reftests with the default "writing-mode" (#001, in previous patch, and #004, in this patch) already pass without the fix. The rest of the tests fail unless the fix is applied.

Comment 6

[Daniel Holbert [:dholbert]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c081d10b6d1d

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 8584376 [details] [diff] [review]
part 1 v2: the fix

r=mats

Comment 8

[Daniel Holbert [:dholbert]]

Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52c1155d7bff
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c6354ab3e22
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e874a1e7a7a

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/52c1155d7bff
https://hg.mozilla.org/mozilla-central/rev/7c6354ab3e22
https://hg.mozilla.org/mozilla-central/rev/2e874a1e7a7a

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8791911ffc95

Comment 11

[Daniel Holbert [:dholbert]]

(^ is fixing spec link in tests to point to the TR instead of the ED spec.)

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8791911ffc95