1711593 - Textfield padding is partly outside of the textfield, with native-themed widgets in Firefox 88

Status: RESOLVED WORKSFORME

(Core :: Widget: Cocoa, defect, P5)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Load attached testcase.
   (The testcase has two textfields that each have 10px of padding. The upper one also has a custom color for its right border, which triggers the less-sleek-looking/appearance:none-style widget theming.)

EXPECTED RESULTS:
The left border of both textfields should be at approximately the same location.

ACTUAL RESULTS:
The left border of the lower textfield (the one where we've only specified padding) is shifted rightwards a fair amount. It looks like the border is drawn in the middle of the padding, or something like that.

I get ACTUAL RESULTS on macOS in Firefox 88, as well as in current Firefox Nightly 90 if I set the pref widget.non-native-theme.enabled to false.

I get EXPECTED RESULTS on other platforms, and in Firefox versions before Firefox 88.

mozregression (with widget.non-native-theme.enabled set to false) tells me that this was a regression from bug 1077365. Regression pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=70178cdbf69fc759fcee2aa1a8d1f16307ad5576&tochange=c541555b90cbbb1a1f0f3776f6c377b6c9a767f1

Having said that: it looks like we're shipping widget.non-native-theme.enabled=true in Firefox 89 and newer, in bug 1697053; and that seems to work around this [hence, marking this as depending-on that bug]. So I'm not sure how much this bug really matters, in light of that fact.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

(Thanks to Schalk for bringing up this issue in the first place and providing the testcase.)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot (Firefox 88 on top, Firefox Nightly 90 on bottom)

Here's a screenshot showing the issue. The top browser is Firefox 88 (affected), the bottom is Firefox Nightly 90 (unaffected, only by virtue of having the non-native-theme pref set to true).

Comment 4

[Daniel Holbert [:dholbert]]

ni=mstange to be sure he's aware of this (unintended?) behavior-change from bug 1077365, and to weigh in on whether we need to care at all about this bug, given that our new non-native-theme isn't affected by this.

Comment 5

[Markus Stange [:mstange]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

Having said that: it looks like we're shipping widget.non-native-theme.enabled=true in Firefox 89 and newer, in bug 1697053; and that seems to work around this [hence, marking this as depending-on that bug]. So I'm not sure how much this bug really matters, in light of that fact.

That's right. Thanks for filing this bug, but given that Firefox 88 is already out and Firefox 89 fixes it, I'm inclined to wontfix this bug. It's unfortunate that it slipped into release - it's one of the risks of having Nightly-only default pref flips.

We could back out the offending patch from 88 in a dot release if this is causing severe visual oddities on real world website.

(In reply to Daniel Holbert [:dholbert] from comment #4)

ni=mstange to be sure he's aware of this (unintended?) behavior-change from bug 1077365

It is slightly unintended but not worth worrying about, in my opinion - this code is now only used in browser UI windows like the library window, and we only need to care about this bug if it's causing visual oddities in those pieces of UI.

Comment 6

[Gregory Pappas [:gregp]]

Should this be closed? It's no longer possible to disable non-native theming of widgets in web content (bug 1848899)

Comment 7

[Daniel Holbert [:dholbert]]

Yup! Thanks.