[webkit-reviews] review granted: [Bug 151234] visibilitychange:hidden doesn't fire during page navigations : [Attachment 409708] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 151234: visibilitychange:hidden doesn't fire during page navigations
https://bugs.webkit.org/show_bug.cgi?id=151234

Attachment 409708: Patch

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




--- Comment #14 from Darin Adler <darin at apple.com> ---
Comment on attachment 409708
  --> https://bugs.webkit.org/attachment.cgi?id=409708
Patch

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

> Source/WebCore/loader/FrameLoader.cpp:3278
> +	       //  Dispatch visibilitychange event.

I don’t think we need this comment, because the code says dispatchEvent,
visibilitychangeEvent. If we want to make that more readable I suggest helper
functions to get rid of some of the distracting additional items.

On the other hand I think it would be worthwhile to comment about why we are
overriding the visibility state. That doesn't seem obvious to me. Possible
topics would be why we need to override it now, why we didn’t do it earlier,
and why it’s OK to leave it that way permanently.