Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to prevent font fetching #424

Closed
jaydev opened this issue Jan 31, 2018 · 16 comments
Closed

Option to prevent font fetching #424

jaydev opened this issue Jan 31, 2018 · 16 comments

Comments

@jaydev
Copy link

jaydev commented Jan 31, 2018

I am using Verdana on my site, which is a web safe font, so it does not need to be fetched from Google Fonts. Verdana is also not available on Google Fonts, so I get a 403 error every time my credit card form is rendered. There should be an option in the Recurly.js styling to set the font but prevent its external loading.

@rathpc
Copy link

rathpc commented Feb 9, 2018

I second this. I would like the option to avoid fetching fonts so that the fields fall back to the rest of teh site default. Passing an empty string currently results in the desired effect but the unnecessary console errors on a failed fetch to googleapis is pretty sloppy.

Please implement a conditional flag for this, thank you!

@rathpc
Copy link

rathpc commented Feb 20, 2018

@chrissrogers Is there any development timeline you would be able to elude to for this enhancement?

@chrissrogers
Copy link
Member

Hey! Sorry, I don't have a timeline right now -- I can say we will get to it, though.

@chrissrogers
Copy link
Member

Just an update here: We have this implemented on our hosted field components and are proceeding with testing. I don't have an exact release timeline, but it should go out within the next couple weeks.

@rathpc
Copy link

rathpc commented Feb 24, 2018

That is awesome news thank you for the update!

@chrissrogers
Copy link
Member

Just letting you know that this has been deployed today. Thanks for the feature request!

@chrissrogers
Copy link
Member

And to explain the update a bit: there is no new option to prevent font fetching, but there is a detection filter that will only pull from Google Fonts if any of the specified fonts are not recognized as web safe. So no new configuration will be needed.

@rathpc
Copy link

rathpc commented Mar 14, 2018

@chrissrogers just so I understand, if I pass an empty string this will no longer try to fetch that from Google? I think your statement above is flip flopped so I just wanted to clarify.

@chrissrogers
Copy link
Member

@rathpc Passing an empty string isn't recommended, as it will use the default browser font family due to the field being in an isolated iframe, sandboxed from the style properties of the parent page. I recommend setting the fontFamily specifically to match your site's input font family.

You could programatically set it according to the computed font family of another input by doing something like

recurly.configure({
  // ...
  fields: {
    all: {
      fontFamily: window.getComputedStyle(myInputElement).getPropertyValue('font-family')
    }
  }
  // ...
});

@rathpc
Copy link

rathpc commented Mar 15, 2018

@chrissrogers Ok, so in the case of setting the fontFamily to a font which GoogleFonts does not host, what would happen in that instance? Previously it would throw an error because it was unable to fetch a font it did not contain within its library.

The reason I mentioned the empty string is that inadvertently it seemed to actually cause the hosted fields to fallback to the parent containers font rather than a browser default. This ended up resulting in the desired effect because we wanted that font to be used in the first place but it is not a font available in Google Fonts. So when we had set the fontFamily to the "correct" font it would then error and use a browser fallback.

Strange behavior indeed but for some reason it had actually been working to an extent with the exception of throwing errors that it could not fetch an empty font name.

@chrissrogers
Copy link
Member

In that instance, it would attempt to retrieve the font, and then if it's not available, will simply assign the string to the fontFamily attribute. It will still result in the 404 responses from Google, but the impact to runtime performance is small. Is the font you're using not a web safe font, but one that your target machines tend to have?

That's very curious about the cascading behavior with an empty string. Not something I would expect -- at least in Chrome, it will assign the system-ui font, irrespective of the parent document's font. In which browser are you seeing this behavior?

Ultimately whatever you do set the fontFamily to, it will be set in the hosted field css property, so if Google fails to retrieve the font, but the machine has the font already, it will display as expected.

@rathpc
Copy link

rathpc commented Mar 15, 2018

The font we are using is a customized font library which we load from our cdn and assign in our scss files via a font-face declaration.

The empty string behavior was experienced in Chrome. If you inspect the iframe dom input field, the font-family is listed as system-ui, however if you inspect the iframe element itself it lists the parent containers font-family. I even went so far as to screenshot the hosted field vs. a local field with the same text and overlay them in photoshop to verify it is in fact the same font. There is the slightest variation in letter-spacing and kerning which we were able to adjust to make them as close as possible to matching using additional properties in the recurly config.

Right now we just get this error for every field on the page during load
GET https://fonts.googleapis.com/css?family= net::ERR_ABORTED - hosted-field.js:12

Still loads the page but obviously not ideal to have console errors on a production page responsible for handling transactions.

@rathpc
Copy link

rathpc commented Mar 15, 2018

@chrissrogers Maybe I am seeking the wrong solution for the problem we are experiencing however, and if this is better suited for a new filed issue please let me know.

Instead of trying to disable fetching or anything of that nature, is there any possibility of adding a configuration option to specify a url for custom font files that a particular organization may be using that are not web-safe OR included on Google Fonts?

This would then eliminate any odd cascading repercussions, errors or false positives when it comes to declaring a fontFamily property. The hosted fields would have direct access to the external font to use thus allowing an all-inclusive solution for any font choice.

@chrissrogers
Copy link
Member

We would need to do some research if such a thing is feasible with PCI DSS SAQ A compliance restrictions. Generally, third party resources are strictly forbidden on documents that accept credit card information, unless the third party is strictly vetted and trusted (which is the case with Google). We can't allow a third party stylesheet with a fontface descriptor to be loaded, and we may not even be able to load a font resource within a pre-configured fontface descriptor, since that could possibly introduce a potential attack vector. It's unfortunate we have to work within these restrictions, but security is of course our primary concern.

I think perhaps the option to disable font fetching altogether is the best way to dispel the error messages when sending an invalid font to google fonts. While it's true that the resource misses are inconsequential to the performance of the page, I definitely understand that 'the optics' of it aren't great for a customer with their console open.

Do you have a live demo of your checkout scenario that I can look through? Could help me completely grok the setup and see if that is indeed the best solution.

@rathpc
Copy link

rathpc commented Mar 15, 2018

@chrissrogers I will be emailing our contact in your support team so that we can take this offline and continue the discussion that way for privacy/security reasons. Thank you.

@chrissrogers
Copy link
Member

100% 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants