Closed Bug 1687682 Opened 3 years ago Closed 3 years ago

[UK][marksandspencer.com] Autofill highlight overlaps Card Type logo detected by the field

Categories

(Toolkit :: Form Autofill, defect, P3)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
94 Branch
Tracking Status
firefox84 --- disabled
firefox85 --- disabled
firefox86 --- disabled
firefox94 --- verified

People

(Reporter: tbabos, Assigned: emilio)

References

(Blocks 1 open bug, Regressed 2 open bugs, )

Details

(Keywords: polish)

Attachments

(2 files)

Affected Versions:

  • All latest Firefox versions using force enable (Nightly 86, Beta RC 85.0, Release 84.0.2)

Tested on:

  • MacOS 10.15
  • Windows 10 x64

Prerequisites:

  • browser.search.region UK
  • extensions.formautofill.supportedCountries UK
  • download latest Firefox Nightly with region locale (switch extensions.formautofill.available to "on" for testing non-Nightly builds)

Steps to Reproduce:

  1. Launch Firefox
  2. Go to and reach the payment form marksandspencer.com
  3. Using autofill feature, select 1 saved CC entry - observe Bug 1
  4. Clear the form and toggle autofill once again from the Expiry month field - observe Bug 2

Expected Results:

  1. Autofill highlight should not overlap the card type logo identified by the field
  2. Autofilling the month field once again should pass field input validation

Actual Results:

  1. Bug 1: The card type logo is overlapped
  2. Bug 2: Autofilling the month once again will not pass field input validation

Notes:

  • Severity: S4 severity, more like a visual issue. The exp month issue is also reproducible on Chrome, seems to be site related
  • Reproducible on Chrome?: No (for the card type logo) Yes (for exp date)
  • Regression-range: reproducible on all latest versions, this site was never tested before, can look for regression-range if need be
Keywords: polish
Priority: -- → P3

As for Bug 1, the card logo overlapping issue, it looks like we've historically used the "filter" CSS property over "background-color" which is what Chrome uses (and probably other browsers to be honest). It looks like this filter was implemented back in Bug 1355438, but I don't see any context why filter was chosen over some alternative method. I believe switching out this filter for background-color: beige or the RGB/hex equivalent would fix all of these instances of other elements being present but covered in the input field when previewing and autofilling.

As for Bug 2, the expiry date validating fail issue, this appears to be because the site's validation is expecting a 'keydown' event at some point to run its validation. However, the payment form seems to be an Angular app so the code is minified and not easily traceable. However, since it is reproducible in Chrome and I believe they send more events than we do...there is some other site specific issue that needs to be tackled.

I think we could ignore bug 2 and focus on bug 1 for this issue.

Assignee: nobody → tgiles

According to Bug 740979, there are two main reasons we went with filter over background-color:

  1. Since the state of ":autofill" changes frequently while doing the preview in Form Autofill, UX doesn't want the elements' appearance to change that much.

  2. we chose to use CSS filter instead, which causes users unable to override the color of autofilled elements by setting the CSS background-color with this pseudo-class (unless they realize that they need to override it by setting CSS filter).

It seems like the historic approach was to prevent our autofill styling from being changed but without using !important in the user agent style sheet. If we switch out the filter for background-color, a website's style sheet would be able to override the browser's autofill styling...which is happening over in Bug 1729745, but for the color of the text and not the background color.

At first glance, it looks like we have two solutions for this issue: 1, swap out filter for an equivalent background-color. 2, swap out filter for background-color and make background color !important.

For 1), we already allow websites to override :autofill styling based on other examples in the wild. This, to me at least, seems to match the intent of the :autofill selector anyway...allowing site authors to add styling to inputs when these inputs are autofilled. This may result in more site specific bugs being filed since I have no idea how many websites use the :autofill selector (other than bandcamp at least...and I'm not sure if bandcamp's case is due to JQuery or not). The fix would be straightforward for websites, remove :autofill from their style sheets. I can imagine this being risky though and being the "final straw that broke the camel's back" for users.

For 2), Chrome (and other browsers) prevent their autofill styles from being overwritten by using !important which ensures consistent styling on all pages. Of course, there is a real possibility that this forced styling clashes with the overall design of a website, but ensures a consistent experience with autofilled inputs. Forcing permanent styling like this, again to me at least, seems to go against the intent of the :autofill selector. It seems unreasonable to expose this selector and then immediately close it off by using !important on the selector's rules. As noted on both the MDN article on autofill as well as webkit's bug tracker for allowing site authors to override autofill styles, a webpage can still get around these !important styles via JS hacks.

My personal opinion is that we go with 1) and swap out filter for background-color without using !important. To me, this matches the spirit of the :autofill selector and allows web developers to customize their user experience for autofilled inputs, on Firefox at least. However, I don't have a strong opinion and could easily be swayed to 2) for the fact that enforcing our styles via !important would ensure a consistent experience everywhere (and prevent future instances of Bug 1729745.

Hey :emilio (sorry for all the NIs!), do you have any strong opinion on changing "input:autofill" from using filter to using background-color? See Comment #3 for more context. Thanks for your time!

Flags: needinfo?(emilio)

No problem!

For 2), Chrome (and other browsers) prevent their autofill styles from being overwritten by using !important which ensures consistent styling on all pages. Of course, there is a real possibility that this forced styling clashes with the overall design of a website, but ensures a consistent experience with autofilled inputs. Forcing permanent styling like this, again to me at least, seems to go against the intent of the :autofill selector. It seems unreasonable to expose this selector and then immediately close it off by using !important on the selector's rules. As noted on both the MDN article on autofill as well as webkit's bug tracker for allowing site authors to override autofill styles, a webpage can still get around these !important styles via JS hacks.

To be fair, :autofill still is useful for some other things (like positioning the label of the input etc, netflix does this for example). But yeah, you described the problem well, the issue with using background-color is that then we'd likely have to !important it and the relevant color declaration, or alternatively special-casing it in some other way, since it's common for authors to specify the background color or color on their own.

Doing that would fix some other bugs like bug 1727950 (see comments there, I believe this is basically a dupe), so it might be reasonable (though unfortunate) given we're seeing a fair number of similar bugs.

Another thing that might work a bit better than background-color: <something> !important; color: <something> !important; is perhaps a semi-transparent background-image?

That should hopefully be less overridden by content than background-color, and will not change the z-index like filter does. Wdyt of trying something like that? The key of it being semi-transparent is that it shouldn't cause so many contrast issues as an opaque background would otherwise (as it'd blend with the original background), and thus we don't need !important on color to guarantee contrast.

Flags: needinfo?(emilio)

Let me try something like that.

Ah! So in trying to implement this, I see now why filter is used. filter allows it to work with "native" form controls. However that should be workable now that we no longer use native form controls (as drawn by the OS) in content.

Assignee: tgiles → emilio

With the non-native theme we don't need filter for this to affect
"native" inputs, we can just implement the logic in nsNativeBasicTheme
instead.

A bit unfortunate that we need that special-case, but it seems better
than using filter, which can break websites due to it creating an
stacking context.

I think there are tests that I need to adjust for this change, but if
not I'll write some.

Keep the current behavior behind a pref just in case.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/282b96702ce9
Make autofill use a semi-transparent background-image rather than filter. r=mstange,tgiles
Backout by imoraru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e61d7417c053
Backed out changeset 282b96702ce9 for causing multiple mochitest failures.

(In reply to Tim Giles [:tgiles] from comment #3)

  1. Since the state of ":autofill" changes frequently while doing the preview in Form Autofill, UX doesn't want the elements' appearance to change that much.

I believe this was referring both to the fact that it previously retained the native control appearance (as mentioned in comment 7) but also that it better handles custom website colours applied to their fields regardless of the autofill state. This patch reduces the contrast ratio to only WCAG 2.0 AA levels (4.24:1) when an input's default style is white text on a black background.

  1. we chose to use CSS filter instead, which causes users unable to override the color of autofilled elements by setting the CSS background-color with this pseudo-class (unless they realize that they need to override it by setting CSS filter).

A reason for not wanting to make it possible/easy for sites to change the style for autofilled fields is because some sites don't like the browser changing the colour at all and will simply remove the UA :autofill styling, thus leading to a poor UX for our users as they may not understand where the filled data comes from (e.g. for autofill without user interaction via the password manager).

I think it's worth trying this out in the sense that websites can already override it today by setting filter: none, and using background-color fixes this and other z-order bugs. If you'd rather not let websites override the autofill colors, we can use !important + an opaque background-color / color, or something of that sort. But that should probably be a separate bug.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b60e8c796f87
Make autofill use a semi-transparent background-image rather than filter. r=mstange,tgiles
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch
Flags: qe-verify+

Hello Emilio and Tim,

So only Bug 1 was fixed? I am asking because Bug 2 is still reproducing.

Bug 1: The card type logo is overlapped
Bug 2: Autofilling the month once again will not pass field input validation

And if so should we close this issue as fixed for Bug1 and log a separate one for Bug2?

Thank you.

Flags: needinfo?(tgiles)
Flags: needinfo?(emilio)

Hey Monica, yeah Bug 1 was fixed. Bug 2 seems like a site specific problem on their end and not on the browser end. We could file a separate bug for Bug 2, but I don't think there's anything actionable on our end. I guess knowing if the problem appears in Safari would confirm that Bug 2 is in fact a site specific bug.

:emilio, don't know what your opinion is on this but I'd be glad to hear it

Flags: needinfo?(tgiles)

Yeah same. Bug 2 fails on other browsers then I wouldn't spend time on it. If it doesn't it may be worth digging into why.

Flags: needinfo?(emilio)

Verified Bug 1 is fixed on Windows 10/Ubuntu 20.04 (Nightly - 95.0a1(13Oct), Beta (94.0b5) )and Mac 11.5.2 Beta (94.0b5).

Bug 2 still reproduces on Firefox but also on Chrome.
Closing issue based on previous comments #18 and #19.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

IMO no possible override this new background for pure html login forms - tested by Stylys addon and legacy userContent.css. Works after add own border for any input on any page.

Anybody tested change to shadow-box in Firefox 94+?

input:autofill {
    box-shadow: 0 0 0 100px #fffff0 inset !important;
}

Have same limitations for override by Stylus or userContent.css?

I tested by repack omni.ja and box-shadow completly does not work with pure html login form :-(

So I must remember change layout.css.autofill.background to false in user.js...

Blocks: 1738608

(In reply to krystian3w from comment #22)

Bug 1738608 should cover that.

Regressions: 1739988
Regressions: 1739995
Regressions: 1740264
Regressions: 1740669

Bug 2 still reproduces on Firefox but also on Chrome.
Closing issue based on previous comments #18 and #19.
https://dzooq.qa/products/platinum-plated-mom-pendant

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: