[Webkit-unassigned] [Bug 56682] Opening link with unspecific hash in a new tab (except context menu) twice in a row results in hash for current window changing

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #87157|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #5 from Adam Barth <abarth at webkit.org>  2011-03-28 16:28:21 PST ---
(From update of attachment 87157)
View in context: https://bugs.webkit.org/attachment.cgi?id=87157&action=review

This patch lacks a test.  Generally speaking, every patch requires a test.

> Source/WebCore/loader/PolicyChecker.cpp:69
> +        const UIEventWithKeyState* keyStateEvent = findEventWithKeyState(const_cast<Event*>(action.event()));

const_cast ?  That doesn't seem right.

> Source/WebCore/loader/PolicyChecker.cpp:74
> +        if (!(keyStateEvent && (keyStateEvent->ctrlKey() || keyStateEvent->metaKey()))) {
> +            function(argument, request, 0, true);
> +            loader->setLastCheckedRequest(request);
> +            return;
> +        }

PolicyChecker shouldn't know anything about ctrlKey or metaKey.  I think this patch is improperly factored.

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