1782385 - <input> element with size attribute is too wide

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Šime Vidas]

Attachment: input-size-text.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

1. Open the attached test page

Actual results:

The second input is wide enough to fit four characters.

Expected results:

The second input should be roughly one character wide because of the size=1 attribute. This is true in Chrome and Safari.

Comment 1

[Šime Vidas]

Attachment: Screenshot of test page in Firefox, Safari, and Chrome

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Form Controls' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

This looks extremely suspicious, specially since we size correctly if you do something like font-family: monospace: https://searchfox.org/mozilla-central/rev/92e8568bbe7c8bf64f7a8ee958291877d960d7d8/layout/forms/nsTextControlFrame.cpp#209-232

Would need to check what other browsers do exactly. Adding more slack so that the longest character of the font fits makes sense to me tho.

Comment 4

[Michal Čaplygin [:myf]]

Attachment: screenshot-windows-canary-chrome-edge-firefox-nightly-explorer-legacy.png

Screenshot of the testcase in various Windows browsers (see Names and versions typed in URL bars).

Widest inputs are in Edge and Chrome, then Firefox and IE11, and narrowest in "some Playwright WebKit" (not sure if relevant).

IE 11 has little "x" button for wiping value that appears in focused input, but only in the first one, the size=1 one does not get it. It almost seems there is a reserved space for it in all browsers, but none of them use it.

Comment 5

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

The severity field is not set for this bug.
:boris, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Šime Vidas from comment #0)

Expected results:
The second input should be roughly one character wide because of the size=1 attribute. This is true in Chrome and Safari.

It's true in Chrome and Safari on macOS, but it's not true in Chrome-on-Windows, per screenshot in comment 4. (I just confirmed by testing the attached testcase in Chrome on macOS vs. Windows.)

On my Linux system, Chrome makes the input about 3 characters wide, whereas Firefox Nightly makes it about 6 characters wide. Epiphany (WebKit) makes it about 1 character wide, so it seems WebKit is consistently skinny here.

Presumably some of the differences between platforms & between browsers here are due to font choices. But yeah, I'd echo Emilio that we should probably be doing something simple like making sure the font's widest character fits (plus a little slack), which looks like roughly what WebKit is doing.

Comment 7

[Jonathan Kew [:jfkthame]]

With the examples at https://codepen.io/jfkthame/pen/ExdwLBo, I'm seeing a lot of variability between different browsers (Firefox, Chrome, Safari all give different results) and between different fonts.

One thing that looks fishy to me in the current Gecko code is the test here, where the comment suggests "only do this if we think we have a fixed-width font", but AFAICS the sense of the test is wrong, and it's applying the "to better match IE" fixup in the non-fixed-width case.

Reversing that test to use < instead of > does indeed affect the behavior (in particular, it causes the monospace case to get a little extra width, which seems to be the intent of the code -- though it looks untidy, IMO), but some of the proportional-font cases end up too short....

Comment 8

[Jonathan Kew [:jfkthame]]

There doesn't seem to be a very clear spec for how the size attribute is supposed to behave.

According to MDN, "[f]or password and text, it is a number of characters (or em units) with a default value of 20", which is not much help -- "characters" and "em units" are quite different things, so which does it mean?!

The HTML spec says

The size attribute gives the number of characters that, in a visual rendering, the user agent is to allow the user to see while editing the element's value.

...if the result is a number greater than zero, then the user agent should ensure that at least that many characters are visible.

but this doesn't really work well for proportional fonts.

The difference in behavior between size=15 as an attribute and width: 15ch as a CSS property, which at first glance seems surprising, arises because the ch unit is based on the width of the 0 (zero) character, whereas the size attribute computes a width based on the font's "average character width" (as found in the OS/2 table), which may be quite different.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1782385 - Improve sizing heuristics in nsTextControlFrame::CalcIntrinsicSize. r=#layout-reviewers

As far as I can see, this was always the intent of the code here, and I see something similar
(i.e. a bit of extra width for input elements when using a monospaced font) in Chrome and
Safari, although the exact amount isn't completely consistent.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1782385 - patch 2 - Use zeroWidth as the basis for <input size=N> if it is larger than aveCharWidth for the font. r=#layout-reviewers

This improves behavior for some proportional fonts where the aveCharWidth is relatively small,
but characters like digits (which are quite likely to occur in input text) are wider.

We'll see what reftests have to say about this, but given the vagueness of the spec,
it may not be testable in any very meaningful way.

Depends on D176922

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew [:jfkthame] from comment #7)

One thing that looks fishy to me in the current Gecko code is the test here, where the comment suggests "only do this if we think we have a fixed-width font", but AFAICS the sense of the test is wrong, and it's applying the "to better match IE" fixup in the non-fixed-width case.

Huh.... looking back at the history of this code, I'm having second thoughts here.

Prior to bug 157846, the code here read:

   // To better match IE, take the maximum character width(in twips) and remove
   // 4 pixels add this on as additional padding(internalPadding). But only do
   // this if charMaxAdvance != charWidth; if they are equal, this is almost
   // certainly a fixed-width font.
   if (charWidth != charMaxAdvance) {
     nscoord internalPadding = std::max(0, charMaxAdvance -
                                         nsPresContext::CSSPixelsToAppUnits(4));

so in other words, for fixed-width fonts, use the exact char width; for non-fixed-width, add a bit of extra width ("to match IE"). That seems a plausible heuristic.

In bug 157846, the test was updated from a strictly-not-equals check to require a > 1px discrepancy before this chunk of code is used:

  if (mozilla::Abs(charWidth - charMaxAdvance) >
      (unsigned)nsPresContext::CSSPixelsToAppUnits(1)) {
    nscoord internalPadding =
        std::max(0, charMaxAdvance - nsPresContext::CSSPixelsToAppUnits(4));

but at the same time, the comment was modified from

But only do this if charMaxAdvance != charWidth; if they are equal, this is almost certainly a fixed-width font.

(i.e. if we think we have a fixed-width font, we don't do this padding of the width) to the shorter text

But only do this if we think we have a fixed-width font.

which is the opposite of the original meaning, and of what the code actually does. I believe this reversal of the meaning when simplifying the comment was accidental.

That modified comment was what led me to think the inequality test was reversed; but in fact it's the comment that is wrong.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b415f741122
Improve sizing heuristics in nsTextControlFrame::CalcIntrinsicSize. r=emilio

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/6b415f741122

Comment 14

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-05-07 Nightly build on macOS 12.
Verified as fixed on Firefox 115.0b4(build ID: 20230611180300) and Nightly 116.0a1(build ID: 20230611214645) on macOS 12, Windows 10, Ubuntu 22.