[webkit-reviews] review granted: [Bug 116670] Fix loader/go-back-cached-main-resource.html test : [Attachment 202708] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 23 10:30:59 PDT 2013
Alexey Proskuryakov <ap at webkit.org> has granted Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 116670: Fix loader/go-back-cached-main-resource.html test
https://bugs.webkit.org/show_bug.cgi?id=116670
Attachment 202708: Patch
https://bugs.webkit.org/attachment.cgi?id=202708&action=review
------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=202708&action=review
Getting rid of the timeout is a very welcome improvement, so r=me. Please
consider the suggestion below.
> LayoutTests/loader/resources/first-page.html:21
> +<body onpageshow="registerListeners();">
This is quite misleading. We only need to register listeners on first load,
they are already present when restoring from cache. But the function does more
than its name says, which is why it's necessary to call it every time!
One way to clean this up would be to rename the function to "didShowPage", and
use event.persisted property to check if we are handling the initial load. Or
just use an anonymous function, and register it as window.onpageshow = function
(event) { ... }
More information about the webkit-reviews
mailing list