1566632 - Buttons aren't properly resized for font inflation

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Jan Henning [:JanH]]

Attachment: button_overflowing.png

As of yet, I don't know how button layouting actually works, but from the results it looks like what happens in conjunction with font inflation is something like

1. Determine the ISize of the button using the original (non-inflated) font size of its contents.
2. Actually layout the button text, this time using the inflated font size. The I-size computed at 1. is used as a fixed constraint, so if it turns out that the text actually takes up more space than that (and if font inflation has now increased the effective font size being used, this is guaranteed to happen), we'll try to fix the situation by applying word wrapping and expanding the BSize of the text, though we may of course still end up with longer words overflowing the original ISize even after word-wrapping.
3. The button is being rendered using the ISize from step 1. and the actual BSize of the text after step 2.

With buttons typically using one or at most a few words, the case of longer words therefore overflowing the button (see screenshot) because they cannot wrap around sufficiently happens quite frequently, including e.g. on the current Bugzilla layout.

Comment 1

[Jan Henning [:JanH]]

Attachment: button-reflow-inflation.html

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jwatt, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Jonathan Watt [:jwatt]]

jfkthame is better placed to set priority on this.

Comment 4

[Jonathan Kew [:jfkthame]]

This does seem like a bug, though AIUI font inflation isn't enabled by default these days.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

It is enabled by default on Fenix, IIRC...

Comment 6

[Jan Henning [:JanH]]

(In reply to Jonathan Kew (:jfkthame) from comment #4)

This does seem like a bug, though AIUI font inflation isn't enabled by default these days.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

It is enabled by default on Fenix, IIRC...

It was enabled initially at some point, then it was turned off again a little while ago, but now it's going to be enabled again.

Comment 7

[Jan Henning [:JanH]]

With some inspiration gained from bug 1540176, preventing the button control frame itself from becoming a font inflation container (as it does at the moment by virtue of being display:inline-block by default) would seem a possible solution.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Why are buttons special from any other inline-block?

Comment 9

[Jan Henning [:JanH]]

It seems that unlike <button>s or <input>s, regular e.g. <div>s that are display:inline-block also seem to turn into font inflation flow roots as well.
So while their sizing behaviour in principle seems the same as for a button (i.e. the width is calculated without taking font inflation into account), small amounts of text - where the problem of insufficient width being available even after word-wrapping is more likely - are inflated correspondingly less [1], thereby minimising the occurrence of this problem again.
Buttons on the other hand aren't considered separately in that regard, i.e. their text always gets the full amount of inflation from the surrounding text.

[1] Or not at all if the amount of text inside the block is less than the line threshold.

Comment 10

[Jan Henning [:JanH]]

Attachment: Bug 1566632 - Part 1: Add some tests for button behaviour under font inflation. r?emilio

Buttons with a fixed height are already correctly prevented from inflating, so
their test already passes. Everything else will be done subsequently.

Comment 11

[Jan Henning [:JanH]]

Attachment: Bug 1566632 - Part 1: Add some tests for button behaviour under font inflation. r?emilio

Buttons with a fixed height are already correctly prevented from inflating, so
their test already passes. Everything else will be done subsequently.

Comment 12

[Jan Henning [:JanH]]

Attachment: Bug 1566632 - Part 2: Prevent buttons from becoming font inflation containers. r?emilio

A similar reasoning as in bug 1540176 applies here as well: Pending a possible
rework of the AutoMaybeDisableFontInflation logic (bug 1619749), inflation
containers generally cannot take the (I)size increase of font inflation into
account during shrink-wrapping, and form controls like buttons are particularly
hard hit, as
a) they often contain only small amounts of text and
b) unlike regular "inline-block" elements such as <div>s, they aren't font
inflation flow roots either, and as such are therefore subject to the full
amount of font inflation from their surrounding contents,
so they are more likely to end up with a situation were the inflated text won't
fit the ISize of the button even with word wrapping applied.

Therefore, as a quicker fix, buttons will now also specifically prevented from
becoming font inflation containers.
This also has the added advantage that they thereby become subject to the logic
from bug 708175 - i.e. if the button has a fixed width, we will no now no longer
inflate its text.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58428a798fa7
Part 1: Add some tests for button behaviour under font inflation. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d308abf762b0
Part 2: Prevent buttons from becoming font inflation containers. r=emilio

Comment 14

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/58428a798fa7
https://hg.mozilla.org/mozilla-central/rev/d308abf762b0