Bug 28179

Summary: [Qt] Shortcuts on webpages don't work if the Qt application has the same shortcut
Product: WebKit Reporter: Benjamin Meyer <ben>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Enhancement CC: ap, hausmann, jturcotte, kenneth, tonikitoo
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
possible fix eric: review-

Description Benjamin Meyer 2009-08-11 10:20:21 PDT
Google docs has the shortcut Ctrl+s to save your document, but if the application using QWebPage has a shortcut for Ctrl+S (Save As typically) the QWebPage will not get the key press and the QShortcut will be fired instead.

Manualtest: Using arora where ctrl+s is set to Save As

Notes: This was the Google Docs bug reported in the Arora review on cnet http://crave.cnet.co.uk/software/0,39029471,49303237-3,00.htm
Comment 1 Benjamin Meyer 2009-08-11 10:25:04 PDT
Created attachment 34575 [details]
possible fix
Comment 2 Kenneth Rohde Christiansen 2009-08-11 10:29:56 PDT
Simon had some concerns about web apps being able to "overwrite" shortcuts.

Would be good to see if the HTML5 have a say in this.
Comment 3 Benjamin Meyer 2009-08-11 10:50:20 PDT
These two arora bug reports look like they might also be for this bug:
http://code.google.com/p/arora/issues/detail?id=515
http://code.google.com/p/arora/issues/detail?id=230
Comment 4 Eric Seidel (no email) 2009-08-11 22:33:34 PDT
Comment on attachment 34575 [details]
possible fix

Seems we should have a manual-test for this.  It should be possible to test in DumpRenderTree, but would likely require modification to DRT.
Comment 5 Simon Hausmann 2009-08-12 01:01:22 PDT
I agree that this change a test, even an autotest would be sufficient.

But more generally speaking I admit I don't like this solution: shortcutOverrideEvent is not a substitute for handling key events. Handling it means accepting or rejecting the event, but not processing the actual keys. That belongs into keyPressEvent.

But with this patch it is just a matter of time until someone complains that their application shortcuts are overridden by web pages, and that's harder to control and we have to tell them to re-implement shortcutOverrideEvent?

I would prefer to see Arora handle its shortcuts in a re-implementation of keyPressEvents.

Another option might be to make QWebPage's behaviour of overriding all shortcuts configurable.
Comment 6 Antonio Gomes 2009-08-12 04:54:48 PDT
if i understood this bug correctly, I the path mozilla is taking to support this kind of situation:

in bug https://bugzilla.mozilla.org/show_bug.cgi?id=448602 they implementing a way to verify if there is a default key listener for a given key (in this case control+s), but application could verify that before setting their own, avoiding conflicts in a safe way.

we could consider implementing something similar in webkit ...
Comment 7 Benjamin Meyer 2009-08-12 08:08:32 PDT
Some more notes from looking things up:

The documentation for ShortcutOverride is very slim and I am not exactly sure how it is suppose to be used, when it is called,  or if this is even the Qt way to handle this bug.  Looking through Qt's source it looked like when a widget overrides a global shortcut (like many do such as QLineEdit, QComboBox etc) they should handle this event saying that they should eat the keyevent and not allow a shortcut to.  Is this correct?

If we could know that there is a listener for a key without having to actually pass a key event then we could accept the event on ShortcutOverride without having to send the key event.  This seems like the correct approach to solving this. 

Tested Safari and it behaves as expected; ctrl+s doesn't do Save As, but passes it to the DOM and GoogleDocs so there might be some code in WebKit already to handle this.

Simon, you mentioned that Arora could handle its shortcuts in a re-implementation of
keyPressEvents.  To do this were you thinking to overload ShortcutOverrideEvent, always accept the event and then in the KeyPressEvent if the key isn't accepted walk up the parent tree find all QShortcut children, and test their QKeySequence if it matches call that shortcut and return?
Comment 8 Jocelyn Turcotte 2014-02-03 03:12:58 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.