The Typekit integration module conflicts with Domain Access. When multiple domains are hosted on a single Drupal instance through Domain Access, the only kit used is the one that matches the domain where the import was last executed. This should be generalized so that all kits are imported and then matched up with the domains they correspond to.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chromix’s picture

This is my initial attempt at fixing this issue. The only catch here is that it's assumed there's only one kit used per domain. This can be easily fixed by changing the typekit_api_kit variable to an array.

chromix’s picture

chromix’s picture

I went ahead and recoded this so that the solution is a little more lightweight and works with multiple kits on multiple domains. To get around a situation where some kits should only be used on some sites, I also added a feature to disable unwanted kits. I also added the necessary variable_del statements in the uninstall.

chromix’s picture

Status: Active » Needs review
sreynen’s picture

The kit-saving part of this seems way more complicated than it needs to be. As I read it, this is what's happening currently:

1) on import, save multiple kit IDs to font
2) on import, also save kit IDs to typekit_api_kits variable
3) on preprocess, get the kits out of the fonts
4) on preprocess, also get the kits out of the typekit_api_kits variable

I don't see what we're gaining from #2 and #4.

I also don't understand why typekit_api_disabled_kits exists at all. Typekit requires fonts to be added to a kit and kits already have domain restrictions. Why do we need a second interface for that in Drupal?

More generally, I don't (yet) see this as specific to Domain Access. Isn't the core problem here that there's no post-import consideration of domains at all? Can we not solve that entirely with Typekit's existing domain restrictions without involving Domain Access?

chromix’s picture

The typekit_api_kits variable (which is saved with domain_conf_variable_set() if Domain Access is enabled) exists to store which kits are enabled on a given domain, rather than having to make API calls and then walk all the kits on every page load. The variable doesn't do all that much if Domain Access isn't enabled, but I left it there so that the same logic applies whether or not the module is enabled.

The problem with the old import function was that it only tested whether the kit worked on the current domain, and the import function in this patch still technically has that problem. The logic I added in typekit_api_preprocess_html() exists to check the kit mapping for the current domain (as there could be many domains) and then caches the result for the next page load in typekit_api_kits. It's effectively a lazy domain check. I could definitely take a more proactive approach by just building a list of all possible domains based on Domain Access's settings and checking against that list in the import function.

I do see what you're getting at, and I like the idea of just storing domain assignments in the fonts themselves, as that would eliminate the need for a variable at all. That would also make disabling kits redundant, since the fonts themselves would contain all the information needed.

I'll take another stab at this patch and post it back here. It'll hopefully be a bit shorter :)

Neslee Canil Pinto’s picture

Status: Needs review » Closed (outdated)