Speed up all page loads #4
Conversation
…ationally expensive
| $variables['iiif_server_location'] = \Drupal::config('islandora_iiif.settings')->get('iiif_server'); | ||
| $variables['file_default_scheme'] = \Drupal::config('system.file')->get('default_scheme'); | ||
| $variables['host'] = \Drupal::request()->getSchemeAndHttpHost(); |
There was a problem hiding this comment.
Not a user of this theme (nor derivatives of it), but... could there hypothetically be issues downstream of other hook_preprocess_image() implementations or templates which expect these variables to exist?
... that said... Drupal's backwards compat policy seems to suggest such being internal:
- https://www.drupal.org/about/core/policies/core-change-policies/drupal-8-and-9-frontend-backwards-compatibility-bc-policy#themes
- https://www.drupal.org/about/core/policies/core-change-policies/drupal-8-and-9-frontend-backwards-compatibility-bc-policy#preprocess-variables
- https://www.drupal.org/about/core/policies/core-change-policies/drupal-8-and-9-backwards-compatibility-and-internal-api#themes
- https://www.drupal.org/about/core/policies/core-change-policies/drupal-8-and-9-backwards-compatibility-and-internal-api#preprocess-variables
... probably fine to drop, if they're indeed not being used.
There was a problem hiding this comment.
We're currently running a few sites with these commented out and there are no known issues. Thanks for a close read, though!
|
We should revisit the approach to actually fix this. If we merge this, then we still need good ALT text and that really shouldn't be the filename. Possibly the media query for filename is doing a really bad query and can be fixed... at least that query could pull in a many to one set of files with a given filename which would really bog down the subsequent joins. |
|
When the media has an ALT text in it, it does use that. So the filename thing is just a fallback, and as a fallback I think it's probably acceptable... |
|
This PR is being marked as stale from inactivity and will be automatically closed in 90 days unless further action is taken. If this PR is still relevant please comment. Please also consider attending the weekly Tech Call to discuss the PR |
Looking up "better" alt titles is too computationally expensive.
This increases all page loads dramatically, and does not impact accessibility when ALT tags are present on Drupal media. A more performant solution will be needed if we are going to try to fetch "better" alt tags than filenames when humans have omitted to fill the ALT tag field on media.