Closed Bug 1148294 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

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.
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.
OS: Linux → All
Hardware: x86_64 → All
Attached patch part 1: the fix (obsolete) — Splinter Review
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).
Attachment #8584374 - Flags: review?(mats)
(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'.)
Attachment #8584374 - Attachment is obsolete: true
Attachment #8584374 - Flags: review?(mats)
Attachment #8584376 - Flags: review?(mats)
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')
...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.
Depends on: 1148298
Comment on attachment 8584376 [details] [diff] [review]
part 1 v2: the fix

r=mats
Attachment #8584376 - Flags: review?(mats) → review+
Blocks: 1149383
(^ is fixing spec link in tests to point to the TR instead of the ED spec.)
Depends on: 1282940
You need to log in before you can comment on or make changes to this bug.