Incorrect baseline alignment for flex container with overflow other than visible

RESOLVED FIXED in Firefox 67

Status

()

defect
P3
normal
RESOLVED FIXED
6 years ago
5 months ago

People

(Reporter: kizmarh, Assigned: dholbert)

Tracking

(Blocks 1 bug)

Trunk
mozilla67
Points:
---
Dependency tree / graph
Bug Flags:
webcompat ?

Firefox Tracking Flags

(firefox67 fixed)

Details

(Whiteboard: [webcompat][wptsync upstream])

Attachments

(6 attachments)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.12786 YaBrowser/13.12.1599.12786 Safari/537.36

Steps to reproduce:

1. Add some blocks with `display: inline-flex`
2. Give some of them `overflow` other than `visible`.
3. Use `vertical-align: baseline` for them.

See this example: http://dabblet.com/gist/8886089, note that in all other browsers it behave properly.



Actual results:

Element with `display: inline-flex` and `overflow` other than `visible` have an incorrect baseline.


Expected results:

The baseline for block with `overflow` other than `visible` should be at the first line box's baseline.
Posted file reporter's testcase
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
OS: Mac OS X → All
Product: Firefox → Core
Hardware: x86 → All
Version: 28 Branch → Trunk
See Also: → 969880
Summary: Incorrect baseline alignment for inline-flex blocks with overflow other than visible → Incorrect baseline alignment for 'inline-flex' container with overflow other than visible
Priority: -- → P4
CSS 2.1 is reasonably clear that the concept of last-line baseline does not cross overflow != visible boundaries, with the text:

The baseline of an 'inline-block' is the baseline of its last line box in the normal flow, unless it has either no in-flow line boxes or if its 'overflow' property has a computed value other than 'visible', in which case the baseline is the bottom margin edge.
( http://www.w3.org/TR/CSS21/visudet.html#propdef-vertical-align )

but the concept of first-line baseline does cross overflow != visible boundaries:

The baseline of a cell is the baseline of the first in-flow line box in the cell, or the first in-flow table-row in the cell, whichever comes first. If there is no such line box or table-row, the baseline is the bottom of content edge of the cell box. For the purposes of finding a baseline, in-flow boxes with a scrolling mechanisms (see the 'overflow' property) must be considered as if scrolled to their origin position. Note that the baseline of a cell may end up below its bottom border, see the example below. 
( http://www.w3.org/TR/CSS21/tables.html#height-layout )

Do we have the same bug for table cells?  (Is it filed?)
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #2)
> CSS 2.1 is reasonably clear that the concept of last-line baseline does not
> cross overflow != visible boundaries
[...]
> but the concept of first-line baseline does cross overflow != visible
> boundaries:

Interesting distinction.  So that implies that this is a valid bug, since we use a flex item's first line as the container's baseline.

> Do we have the same bug for table cells?

No, we don't seem to have the same bug with table-cells.  Here's a testcase demonstrating that, with an "overflow:scroll" on a <div> inside of an inline-table. It successfully baseline-aligns (using its first line) with text outside the table.

(side note: I initially tried this with 'overflow' directly set on a table-cell (both <td> and <div style="display:table-cell"), but that didn't work, because apparently 'overflow' has no rendering effect on table-cells. So this testcase has 'overflow' set on a <div>, which gets wrapped in a table-cell.)
Posted file reduced testcase 1
Posted file reference case 1
overflow:hidden may have effect on table cells if there fixed table layout, i.e. table-layout:fixed is set on table.
(In reply to Lev Solntsev from comment #6)
> overflow:hidden may have effect on table cells if there fixed table layout,
> i.e. table-layout:fixed is set on table.

(That doesn't seem to make overflow:hidden or overflow:scroll do anything interesting on my end, from a few attempts with my attached "testcase with 'inline-table'", attachment 8373641 [details])
I guess, the Flexbox spec (http://www.w3.org/TR/css3-flexbox/#flex-baselines) should be mentioned here as well.

Regarding the subject of a bug:

> if the box contributing a baseline has an ‘overflow’ value that allows scrolling, the box must be treated as being in its initial scroll position for the purpose of determining its baseline.
Assignee: nobody → dholbert
Duplicate of this bug: 1036404
Surprisingly, it appears that the experimental implementation of Flexbox 2009 (display: -moz-inline-box) doesn't have this bug: http://dabblet.com/gist/3b91b713f8a6db0a50d2

What is the difference between these implementations relating to baseline? Couldn't the old baseline positioning be just ported into new implementation?
Summary: Incorrect baseline alignment for 'inline-flex' container with overflow other than visible → Incorrect baseline alignment for flex container with overflow other than visible
Duplicate of this bug: 1471942
(In reply to Ilya from comment #10)
> Surprisingly, it appears that the experimental implementation of Flexbox
> 2009 (display: -moz-inline-box) doesn't have this bug: [...]
> 
> What is the difference between these implementations relating to baseline?
> Couldn't the old baseline positioning be just ported into new implementation?

(Sorry for the delayed response)

-moz-inline-box is a XUL thing -- we're better off porting behavior from display:inline-block rather than from XUL box layout.

Also, for the record: https://github.com/webcompat/web-bugs/issues/17781 is effectively an instance of this bug affecting twitter.com.
As discussed on dupe-bug 1471942, this now has clearer spec text (which indicates that inline-block is special for legacy reasons & other scrollable inline boxes should Just Work).  Link:  https://drafts.csswg.org/css-align/#baseline-export
Duplicate of this bug: 1518715
Duplicate of this bug: 1522610

This appears to be causing layout issues on the new twitter design.

I'm taking a look at this.

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the inline-block special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export

Depends on: 1525133
Attachment #9040947 - Attachment description: Bug 969874: Make scrollable frames derive their baseline from their scrolled content (except for inline-block). → Bug 969874: Make scrollable frames derive their baseline from their scrolled content (unless their display value is block-inside). r?mats
Priority: P4 → P3

Here's a screenshot of the Twitter UI that was broken due to this bug (look at the "N Retweets/Likes" baseline alignment at the bottom).

Leftmost is Firefox Nightly (broken), middle is my build with the patch applied, and right is Chrome for reference.

As you can see, the baseline alignment seems to be good (and identical) between patched-Firefox and Chrome.

Attachment #9041614 - Attachment filename: Selection_045.png → screenshot.png
Attachment #9041614 - Attachment is patch: false
Attachment #9041614 - Attachment mime type: text/plain → image/png
Flags: webcompat?
Whiteboard: [webcompat]
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21b74260c8a9
Make scrollable frames derive their baseline from their scrolled content (unless their display value is block-inside). r=mats

Daniel, does this patch really want to use IsBlockFrame() rather than, e.g., IsFrameOfType(eBlock)? IsBlockFrame only returns true for actual nsBlockFrames, not for subclasses like ComboboxDisplayFrame or DetailsFrame.

Flags: needinfo?(dholbert)
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Ah, you're right emilio - the patch broke e.g. this testcase (to now align with the [hidden] last-line baseline of the text inside the details frame): https://jsfiddle.net/xqf1w097/

I should indeed use IsFrameOfType(eBlock) to avoid getting the New Behavior for block subclasses. I'll file a followup on that.

Blocks: 1525628
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/15277 for changes under testing/web-platform/tests
Whiteboard: [webcompat] → [webcompat][wptsync upstream]

[clearing ni, as it's addressed by bug 1525628]

Flags: needinfo?(dholbert)
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.