[Webkit-unassigned] [Bug 47817] FrameLoader::IsProcessingUserGesture is true for JavaScript initiated downloads after click navigation to webpage

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47817


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71391|review?                     |review-
               Flag|                            |




--- Comment #26 from Adam Barth <abarth at webkit.org>  2010-10-20 22:51:47 PST ---
(From update of attachment 71391)
View in context: https://bugs.webkit.org/attachment.cgi?id=71391&action=review

> WebCore/loader/NavigationScheduler.cpp:82
> +    // The derived class can call this method to override the m_wasUserGesture.
> +    void overrideWasUserGesture(bool wasUserGesture) { m_wasUserGesture = wasUserGesture; }

I'd remove this comment and call the function clearUserGesture().  Currently we don't have a use case for passing "true" here, and I'm not sure we will.

> WebCore/loader/NavigationScheduler.cpp:139
> +        // All ScheduledRedirects are not user-initiated, we need to override the m_wasUserGesture to false.
> +        overrideWasUserGesture(false);

I'd remove this comment too.  The test is a better explanation for why this line of code exists.

> LayoutTests/fast/frames/location-redirect-user-gesture.html:11
> +        setTimeout("location.href='data:text/html\,<script>layoutTestController.notifyDone()</" + "script>'", 1000);

The \ isn't needed.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:11
> +    if (1 || window.layoutTestController) {

1 || window.layoutTestController will always be true.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:17
> +        if (2 == sessionStorage.loadCount) {

Please clear session storage after this test so we can reload the test and have it work.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:18
> +            layoutTestController.notifyDone();

Doesn't this race with the refresh?

I feel like we're being cheap.  Why not just add another file and refresh to that file?  Then there isn't a worry about an infinite loop.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.