Skip to content
Snippets Groups Projects

#3167034 "Leverage the 'loading' html attribute to enable lazy-load by default for images"

#3167034 "Leverage the 'loading' html attribute to enable lazy-load by default for images"

Related #3167034

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Edys Meza added 1 commit

    added 1 commit

    • 8cfda822 - Issue 3167034: Adressing heddn's comments

    Compare with previous version

  • Edys Meza
    Edys Meza @edysmp started a thread on an outdated change in commit 8cfda822
  • 82 82 if ($node->nodeName == 'img') {
    83 83 // Without dimensions specified, layout shifts can occur,
    84 84 // which are more noticeable on pages that take some time to load.
    85 list($width, $height) = getimagesize($file->getFileUri());
    86 $node->setAttribute('width', $width);
    87 $node->setAttribute('height', $height);
    88 $node->setAttribute('loading', 'lazy');
    85 // As a result, only mark images as lazy load that have dimensions.
    86 [$width, $height] = @getimagesize($file->getFileUri());
    87 if ($width != NULL && $height != NULL) {
  • Edys Meza added 1 commit

    added 1 commit

    • dab23783 - Issue 3167034: Addressing Google and heddn feedback

    Compare with previous version

  • 79 79 $file = $this->entityRepository->loadEntityByUuid('file', $uuid);
    80 80 if ($file instanceof FileInterface) {
    81 81 $node->setAttribute('src', $file->createFileUrl());
    82 if ($node->nodeName == 'img') {
    83 // Without dimensions specified, layout shifts can occur,
    84 // which are more noticeable on pages that take some time to load.
    85 // As a result, only mark images as lazy load that have dimensions.
    86 [$width, $height] = @getimagesize($file->getFileUri());
  • Edys Meza added 1 commit

    added 1 commit

    • 470e1d71 - Issue 3167034: use isset intead of attributeExists

    Compare with previous version

  • 851 851 $variables['attributes'][$key] = $variables[$key];
    852 852 }
    853 853 }
    854
    855 // Without dimensions specified, layout shifts can occur,
    856 // which are more noticeable on pages that take some time to load.
    857 // As a result, only mark images as lazy load that have dimensions.
    858 if (isset($variables['width'], $variables['height']) && !isset($variables['loading'])) {
    • I did quite the same thing in native_lazy_loading_preprocess_image() (contrib module), but I also checked for emptiness, because I had some use case where it was set but equal to ''. I don't remember exactly why.

      Suggested change
      858 if (isset($variables['width'], $variables['height']) && !isset($variables['loading'])) {
      858 if (
      859 isset($variables['width'], $variables['height'])
      860 && !isset($variables['loading'])
      861 && !empty($variables['width'])
      862 && !empty($variables['height'])
      863 ) {
    • Nothing in core should be adding the empty loading attribute. Do we really need to do the empty check on it? It seems like something a contrib or custom code is introducing garbage data.

      Edited by Lucas Hedding
    • changed this line in version 10 of the diff

    • Please register or sign in to reply
  • Edys Meza added 1 commit

    added 1 commit

    • f47bf829 - Issue 3167034: use image factory

    Compare with previous version

  • Edys Meza added 1 commit

    added 1 commit

    • 77bdbd02 - Issue 3167034: tests loading attribute value can be overriden

    Compare with previous version

  • 26 /**
    27 * Tests that loading attribute is enabled for images.
    28 */
    29 public function testImageLoadingAttribute() {
    30 // Get page under test.
    31 $this->drupalGet('image-lazy-load-test');
    32
    33 // Loading attribute is added when image dimensions has been set.
    34 $this->assertSession()->elementAttributeExists('css', '#with-dimensions img', 'loading');
    35 $this->assertSession()->elementAttributeContains('css', '#with-dimensions img', 'loading', 'lazy');
    36
    37 // Loading attribute with lazy default value can be overriden.
    38 $this->assertSession()->elementAttributeContains('css', '#override-loading-attribute img', 'loading', 'eager');
    39
    40 // Without image dimensions loading attribute is not generated.
    41 $this->assertSession()->elementAttributeContains('css', '#without-dimensions img', 'alt', 'Image lazy load testing image without dimensions');
  • Edys Meza added 135 commits

    added 135 commits

    • 77bdbd02...7e524d3b - 124 commits from branch project:9.1.x
    • 30baa59d - Issue #3167034: Enable the loading html attribute to enable lazy-load by default for images
    • fd22a525 - Issue #3167034: Adding simple test coverage
    • 4df5eddb - Issue #3167034: CKeditor support
    • bf95ad94 - Issue #3167034: Update ImageDimensions asserts
    • 4f628583 - Issue #3167034: fixing more tests.
    • b6a453dd - Issue 3167034: Adressing heddn's comments
    • a9a46d7b - Issue 3167034: Addressing Google and heddn feedback
    • 36da7865 - Issue 3167034: use isset intead of attributeExists
    • d0519240 - Issue 3167034: use image factory
    • 66041322 - Issue 3167034: tests loading attribute value can be overriden
    • ef1bb0f1 - Merge branch '3167034-lazy-load-images' of git.drupal.org:issue/drupal-3167034...

    Compare with previous version

  • Edys Meza added 1 commit

    added 1 commit

    • 2175e1f0 - Issue 3167034: better handling elementAttributeExists assert method

    Compare with previous version

  • Lucas Hedding
    Lucas Hedding @lucashedding started a thread on an outdated change in commit 2175e1f0
  • 32 36
    33 37 // Loading attribute is added when image dimensions has been set.
    34 $this->assertSession()->elementAttributeExists('css', '#with-dimensions img', 'loading');
    35 $this->assertSession()->elementAttributeContains('css', '#with-dimensions img', 'loading', 'lazy');
    38 $assert->elementAttributeExists('css', '#with-dimensions img', 'loading');
    39 $assert->elementAttributeContains('css', '#with-dimensions img', 'loading', 'lazy');
    36 40
    37 41 // Loading attribute with lazy default value can be overriden.
    38 $this->assertSession()->elementAttributeContains('css', '#override-loading-attribute img', 'loading', 'eager');
    42 $assert->elementAttributeContains('css', '#override-loading-attribute img', 'loading', 'eager');
    39 43
    40 44 // Without image dimensions loading attribute is not generated.
    41 $this->assertSession()->elementAttributeContains('css', '#without-dimensions img', 'alt', 'Image lazy load testing image without dimensions');
    42 $this->expectExceptionMessage('The attribute "loading" was not found in the element matching css "#without-dimensions img".');
    43 $this->assertSession()->elementAttributeExists('css', '#without-dimensions img', 'loading');
    45 $this->assertFalse($this->elementAttributeExists($assert, 'css', '#without-dimensions img', 'loading'));
  • Edys Meza added 1 commit

    added 1 commit

    • f59f12f8 - Issue 3167034: do not use try/catch on test

    Compare with previous version

  • 42 50 * The plugin implementation definition.
    43 51 * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
    44 52 * The entity repository.
    53 * @param \Drupal\Core\Image\ImageFactory $image_factory
    54 * The image factory.
    45 55 */
    46 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository) {
    56 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository, ImageFactory $image_factory) {
    47 57 $this->entityRepository = $entity_repository;
    58 $this->imageFactory = $image_factory;
    • as this argument is new it needs deprecated shim and it can't be required

      Suggested change
      62 $this->imageFactory = $image_factory;
      62 if ($image_factory === NULL)
      63 @trigger_error('Calling ' . __METHOD__ . '() without the $image_factory argument is deprecated in drupal:9.1.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3173719', E_USER_DEPRECATED);
      64 $image_factory = \Drupal::service('image.factory');
      65 }
      66 $this->imageFactory = $image_factory;
    • Please register or sign in to reply
  • 42 50 * The plugin implementation definition.
    43 51 * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
    44 52 * The entity repository.
    53 * @param \Drupal\Core\Image\ImageFactory $image_factory
    54 * The image factory.
    45 55 */
    46 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository) {
    56 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository, ImageFactory $image_factory) {
  • Edys Meza added 1 commit

    added 1 commit

    • b9b65be4 - Issue 3167034: Deprecates EditorFileReference construct method

    Compare with previous version

  • 42 50 * The plugin implementation definition.
    43 51 * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
    44 52 * The entity repository.
    53 * @param \Drupal\Core\Image\ImageFactory $image_factory
    54 * The image factory.
    45 55 */
    46 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository) {
    56 public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityRepositoryInterface $entity_repository, ImageFactory $image_factory = NULL) {
    47 57 $this->entityRepository = $entity_repository;
    58 if ($image_factory === NULL) {
    59 @trigger_error('Calling ' . __METHOD__ . '() without the $image_factory argument is deprecated in drupal:9.1.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3173719', E_USER_DEPRECATED);
  • Edys Meza added 1 commit

    added 1 commit

    • 8e3d8771 - Issue 3167034: update deprecation message

    Compare with previous version

  • Edys Meza added 36 commits

    added 36 commits

    • 8e3d8771...cd8213d2 - 35 commits from branch project:9.1.x
    • 13741187 - Merge remote-tracking branch 'origin/9.1.x' into 3167034-lazy-load-images

    Compare with previous version

  • merged

  • Please register or sign in to reply
    Loading