Incorrect baseline alignment for flex container with overflow other than visible

RESOLVED FIXED in Firefox 67

Status

()

defect
P3
normal
RESOLVED FIXED
5 years ago
3 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)

Reporter

Description

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

Comment 1

5 years ago
Posted file reporter's testcase
Assignee

Updated

5 years ago
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
OS: Mac OS X → All
Product: Firefox → Core
Hardware: x86 → All
Version: 28 Branch → Trunk
Assignee

Updated

5 years ago
See Also: → 969880
Assignee

Updated

5 years ago
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
Assignee

Updated

5 years ago
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?)
Assignee

Comment 3

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

Comment 4

5 years ago
Posted file reduced testcase 1
Assignee

Comment 5

5 years ago
Posted file reference case 1

Comment 6

5 years ago
overflow:hidden may have effect on table cells if there fixed table layout, i.e. table-layout:fixed is set on table.
Assignee

Comment 7

5 years ago
(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])
Reporter

Comment 8

5 years ago
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

Updated

5 years ago
Assignee: nobody → dholbert
Assignee

Updated

5 years ago
Duplicate of this bug: 1036404

Comment 10

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

Updated

4 years ago
Summary: Incorrect baseline alignment for 'inline-flex' container with overflow other than visible → Incorrect baseline alignment for flex container with overflow other than visible
Assignee

Updated

10 months ago
Duplicate of this bug: 1471942
Assignee

Comment 12

10 months ago
(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.
Assignee

Comment 13

9 months ago
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
Assignee

Updated

4 months ago
Duplicate of this bug: 1518715
Assignee

Updated

4 months ago
Duplicate of this bug: 1522610

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

Assignee

Comment 17

4 months ago

I'm taking a look at this.

Assignee

Comment 18

4 months ago

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

Assignee

Updated

4 months ago
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
Assignee

Updated

3 months ago
Priority: P4 → P3
Assignee

Comment 20

3 months ago

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.

Assignee

Updated

3 months ago
Attachment #9041614 - Attachment filename: Selection_045.png → screenshot.png
Assignee

Updated

3 months ago
Attachment #9041614 - Attachment is patch: false
Attachment #9041614 - Attachment mime type: text/plain → image/png
Flags: webcompat?
Whiteboard: [webcompat]

Comment 21

3 months ago
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)

Comment 23

3 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee

Comment 24

3 months ago

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.

Assignee

Updated

3 months ago
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]
Assignee

Comment 26

3 months ago

[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.