[webkit-reviews] review granted: [Bug 169995] CMD+R / CMD+Q is considered as user interaction and beforeunload alert is shown : [Attachment 306093] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 169995: CMD+R / CMD+Q is considered as user interaction and beforeunload
alert is shown
https://bugs.webkit.org/show_bug.cgi?id=169995

Attachment 306093: Patch

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




--- Comment #17 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 306093
  --> https://bugs.webkit.org/attachment.cgi?id=306093
Patch

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

r=me

> Source/WebCore/dom/Document.h:1156
> +    void markUserDidInteractWithPage() { m_userDidInteractWithPage = true; }

I would just use normal "set" syntax here: setUserDidInteractWithPage(bool).

> Source/WebCore/page/EventHandler.cpp:3153
> +    // Delay updating document flag indicating the user has interacted with
the page until the KeyUp so we do not
> +    // treat keyboard shortcuts such as CMD+R / CMD+Q as user interaction
with the page.

It's odd to special case Q and R. There are other user actions that we don't
treat as interaction with the page -- such as Cmd-L to focus the location field
and change location. I think it would be better to use the set of all key
combinations that Safari treats as special.