Skip to content

Conversation

@ebinnion
Copy link
Contributor

@ebinnion ebinnion commented Feb 20, 2018

Fixes #8821

When we first worked on Lazy Images, we were using the W3TC IntersectionObserver polyfill, but near the end, we moved to another polyfill because the W3TC version had conflicting licenses. Specifically, in one case they were using an Apache 2.0 license which isn't GPL compatible. You can see more discussion about that in #8093.

As of w3c/IntersectionObserver@4a9a908 and w3c/IntersectionObserver@b025731, they have updated their licenses to "W3C SOFTWARE AND DOCUMENT NOTICE AND LICENSE", which is GPL compatible!

Since this is the one being worked on by W3C, I imagine that it's the most up-to-date and well tested. Using this polyfill fixes the Twitter embed issue reported in #8821.

We should do regression testing in other browsers before shipping.

Note: This is a large PR, but it is essentially copying and pasting a new polyfill in. 😄

To test:

  • Checkout branch
  • run yarn build to build the minified files
  • Ensure that license appears in the minified lazy images JavaScript files
  • Create several posts with images
  • Turn lazy images on
  • Ensure that images load lazily as expected
  • Set define( 'SCRIPT_DEBUG', true ); in wp-config.php or an integration plugin, which will use the non-minified scripts
  • Ensure that images load lazily as expected

Changelog entry

  • We updated the library we use as IntersectionObserver for Lazy Images. We now use the W3C IntersectionObserver polyfill.

@ebinnion ebinnion added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Feature] Lazy Images labels Feb 20, 2018
@ebinnion ebinnion added this to the 5.9 milestone Feb 20, 2018
@ebinnion ebinnion self-assigned this Feb 20, 2018
@ebinnion ebinnion requested a review from a team as a code owner February 20, 2018 23:21
Copy link
Contributor

@gravityrail gravityrail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ebinnion
Copy link
Contributor Author

I've tested in Chrome, Firefox, Safari, and back to IE9 without any issues. This should be good to go. 👍

Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 23, 2018
@oskosk oskosk merged commit 83a88e1 into master Feb 23, 2018
@oskosk oskosk deleted the fix/lazy-images-8821 branch February 23, 2018 14:50
oskosk added a commit that referenced this pull request Feb 27, 2018
oskosk added a commit that referenced this pull request Feb 27, 2018
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Lazy Images [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lazy Images: Prevents Embedded Tweets from Displaying Properly In Safari

6 participants