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

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks 1 bug)

Trunk
mozilla39
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

4 years ago
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.
Assignee

Comment 1

4 years ago
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.
Assignee

Updated

4 years ago
OS: Linux → All
Hardware: x86_64 → All
Assignee

Comment 2

4 years ago
Posted 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)
Assignee

Comment 3

4 years ago
(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)
Assignee

Comment 4

4 years ago
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')
Assignee

Comment 5

4 years ago
...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.
Assignee

Updated

4 years ago
Depends on: 1148298
Comment on attachment 8584376 [details] [diff] [review]
part 1 v2: the fix

r=mats
Attachment #8584376 - Flags: review?(mats) → review+
Assignee

Updated

4 years ago
Blocks: 1149383
Assignee

Comment 11

4 years ago
(^ 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.