[webkit-reviews] review granted: [Bug 198530] Improve performance of the lazy image loader : [Attachment 371285] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Jon Davis <jond at apple.com>'s
request for review:
Bug 198530: Improve performance of the lazy image loader
https://bugs.webkit.org/show_bug.cgi?id=198530

Attachment 371285: Patch

https://bugs.webkit.org/attachment.cgi?id=371285&action=review




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 371285
  --> https://bugs.webkit.org/attachment.cgi?id=371285
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371285&action=review

r=me, neat!  It may be worth a simple `if (window.IntersectionObserver)` check
around (or as an early return in) `lazyLoadImages` so we don't throw an error
if it's not supported.

> Websites/webkit.org/ChangeLog:-101
> -	   

I don't think we normally go back and edit ChangeLog entries, no matter how
"bad" they may be.

> Websites/webkit.org/wp-content/themes/webkit/scripts/global.js:-2
> -    var openClass = ' open-menu',

A comment explaining why this isn't needed would be nice :)

> Websites/webkit.org/wp-content/themes/webkit/scripts/global.js:6
> +	   for (let table of tables) {

Are we using `let` now??!? =D

> Websites/webkit.org/wp-content/themes/webkit/scripts/global.js:22
> +	   if (backgroundImages.length == 0)

NIT: you could use `!backgroundImages.length` instead