1673895 - No control over file input label padding

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement)

Description

[Martijn Cuppens]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.99 Safari/537.36

Steps to reproduce:

In a file input, there is a spacing of 5px between the button and it's label, see https://searchfox.org/mozilla-central/rev/0b7007a23bc16c857f894140e12f307bfeef2fdd/layout/style/res/forms.css#494

Actual results:

Since this padding is set on the label, and not on the button, we're unable to undo this spacing by anything else then applying a Firefox specific hack.

Expected results:

By moving setting this spacing as a margin on the button (::file-selector-button), we will be able to easily control this spacing, since we can theme the button with a pseudo element selector.

Ref (see subtract($input-padding-x, 5px), and BS function which turns it into a calc() function):

https://github.com/twbs/bootstrap/pull/31955/files#diff-749f2947fecd5d2d4d1aa6dde651d54f33377d23e415591e53d632ab4be91169

Comment 1

[Martijn Cuppens]

FYI, Chrome adds this spacing to the button too:

input[type="file" i]::-webkit-file-upload-button {
  ...
  margin-inline-end: 4px;
  ...
}

Comment 2

[Hani Yacoub, Desktop QA]

Assigning "Core: Layout: Form Control" component.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Seems like a sensible request.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1673895 - Allow customizing the spacing between the button and the label of a file input. r=#layout-reviewers

This should have no behavior changes, but allows authors to customize
the amount of spacing between the label and the button of a file control
by applying it as margin of the file selector pseudo rather than as
padding of the label.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

So, one issue with this is that it'd break the very same code you've linked to, right?

That seems fairly unfortunate :(

Comment 6

[Emilio Cobos Álvarez (:emilio)]

We could uplift this change without much trouble to beta 83 or such, but... how many people are going to be using the code in comment 0? We wouldn't want to break bootstrap, what is its release schedule?

Comment 7

[Martijn Cuppens]

So, one issue with this is that it'd break the very same code you've linked to, right?

Yeah, but Bootstrap 5 is still in alpha fase, so we can easily change this if needed. We're planning on releasing v5 in december (no specific date yet).

Comment 8

[Martijn Cuppens]

We wouldn't want to break bootstrap

Well, it would will look like https://github.com/twbs/bootstrap/pull/31955#issuecomment-716101054 (increased padding between button and label) if it lands, which still looks ok. I'll take care of a pull request to sync Bootstrap & Firefox 😉

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Wouldn't it look the other way around, with a negative margin, if we land this patch?

Comment 10

[Martijn Cuppens]

Wouldn't it look the other way around, with a negative margin, if we land this patch?

Ow yeah, you're right. I've checked the result of this and there's still some spacing left than

Anyway, if this lands in FF 83 or 84, the impact of the change will be pretty minimal.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cb86379163f
Allow customizing the spacing between the button and the label of a file input. r=TYLin

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9185320 [details]
Bug 1673895 - Allow customizing the spacing between the button and the label of a file input. r=#layout-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Some future versions of bootstrap may have weird padding in file inputs.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple CSS change that is only visible via a pseudo-element that we shipped in Firefox 82.

This change aligns us with the relevant pseudo-elements in other browsers, to make authors' lives a bit easier, and should have no visible impact on pages that don't use the new pseudo-element yet, pr that do not override the margin in the pseudo-element, which should be the immense majority.

• String changes made/needed: none

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5cb86379163f

Comment 14

[Martijn Cuppens]

@Emilio, I spotted a small typo in https://hg.mozilla.org/mozilla-central/rev/5cb86379163f (missing n):

+<title>CSS Test: margin can be used to shrink the spacing between the file selector button and its label</title>

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9185320 [details]
Bug 1673895 - Allow customizing the spacing between the button and the label of a file input. r=#layout-reviewers

Looks safe as this is a new feature in Firefox 82. Minimal patch with a test and it makes us more compatible with Bootstrap 5 which may ship during the 83 and aligns us with other browsers default CSS, approved for 83 beta 9, thanks.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1673895 - Fix a typo in a test title.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Thanks :)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e78a6814afa
Fix a typo in a test title. DONTBUILD

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/045d0b692318

Comment 20

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/9e78a6814afa