Incorrect baseline alignment for flex container with overflow other than visible

RESOLVED FIXED in Firefox 67

Status

()

P3
normal
RESOLVED FIXED
5 years ago
a month 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: → bug 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

8 months ago
Duplicate of this bug: 1471942
(Assignee)

Comment 12

8 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

7 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

2 months ago
Duplicate of this bug: 1518715
(Assignee)

Updated

2 months ago
Duplicate of this bug: 1522610

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

(Assignee)

Comment 17

2 months ago

I'm taking a look at this.

(Assignee)

Comment 18

2 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

2 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

a month ago
Priority: P4 → P3
(Assignee)

Comment 20

a month 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

a month ago
Attachment #9041614 - Attachment filename: Selection_045.png → screenshot.png
(Assignee)

Updated

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

Comment 21

a month 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

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
(Assignee)

Comment 24

a month 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

a month 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

a month 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.