28179 2009-08-11 10:20:21 -0700 [Qt] Shortcuts on webpages don't work if the Qt application has the same shortcut 2014-02-03 03:12:58 -0800 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) All All RESOLVED INVALID Qt, QtTriaged P2 Enhancement --- 1 ben webkit-unassigned ap hausmann jturcotte kenneth tonikitoo oldest_to_newest 139020 0 ben 2009-08-11 10:20:21 -0700 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 139026 1 34575 ben 2009-08-11 10:25:04 -0700 Created attachment 34575 possible fix 139032 2 kenneth 2009-08-11 10:29:56 -0700 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. 139038 3 ben 2009-08-11 10:50:20 -0700 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 139242 4 34575 eric 2009-08-11 22:33:34 -0700 Comment on attachment 34575 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. 139276 5 hausmann 2009-08-12 01:01:22 -0700 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. 139298 6 tonikitoo 2009-08-12 04:54:48 -0700 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 ... 139323 7 ben 2009-08-12 08:08:32 -0700 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? 975013 8 jturcotte 2014-02-03 03:12:58 -0800 === 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. 34575 2009-08-11 10:25:04 -0700 2010-06-11 12:25:10 -0700 possible fix 0001-2009-08-11-Benjamin-C-Meyer-ben-meyerhome.net.patch text/plain 3083 ben RnJvbSBiYjFiNTA0OTZiMjc4NWU5NDYyMzc5MjUzODE2MTE2ZWJhZTQ0NTY0IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBCZW5qYW1pbiBDIE1leWVyIDxiZW5qYW1pbi5tZXllckB0b3Jj aG1vYmlsZS5jb20+CkRhdGU6IFR1ZSwgMTEgQXVnIDIwMDkgMDg6MTg6NTcgLTA0MDAKU3ViamVj dDogW1BBVENIXSAyMDA5LTA4LTExICBCZW5qYW1pbiBDIE1leWVyICA8YmVuQG1leWVyaG9tZS5u ZXQ+CgogICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKICAgICAgICBQYXNzIGtl eSBldmVudHMgdG8gdGhlIGRvbSBpbiBzaG9ydGN1dE92ZXJyaWRlRXZlbnQgc28gd2Vic2l0ZXMK ICAgICAgICBsaWtlIEdvb2dsZURvY3MgY2FuIGNhdGNoIEN0cmwrUyBiZWZvcmUgYXBwbGljYXRp b24gUVNob3J0Y3V0cyBkby4KICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MjgxNzkKCiAgICAgICAgKiBBcGkvcXdlYnBhZ2UuY3BwOgogICAgICAgIChRV2Vi UGFnZVByaXZhdGU6OmtleVByZXNzRXZlbnQpOgogICAgICAgIChRV2ViUGFnZVByaXZhdGU6OnNo b3J0Y3V0T3ZlcnJpZGVFdmVudCk6Ci0tLQogV2ViS2l0L3F0L0FwaS9xd2VicGFnZS5jcHAgfCAg IDE2ICsrKysrKysrKysrLS0tLS0KIFdlYktpdC9xdC9DaGFuZ2VMb2cgICAgICAgIHwgICAxMiAr KysrKysrKysrKysKIDIgZmlsZXMgY2hhbmdlZCwgMjMgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlv bnMoLSkKCmRpZmYgLS1naXQgYS9XZWJLaXQvcXQvQXBpL3F3ZWJwYWdlLmNwcCBiL1dlYktpdC9x dC9BcGkvcXdlYnBhZ2UuY3BwCmluZGV4IDhlNDAzMzkuLjJjYzA5NzMgMTAwNjQ0Ci0tLSBhL1dl YktpdC9xdC9BcGkvcXdlYnBhZ2UuY3BwCisrKyBiL1dlYktpdC9xdC9BcGkvcXdlYnBhZ2UuY3Bw CkBAIC04MDUsMTEgKzgwNSw5IEBAIHZvaWQgUVdlYlBhZ2VQcml2YXRlOjprZXlQcmVzc0V2ZW50 KFFLZXlFdmVudCAqZXYpCiAgICAgYm9vbCBoYW5kbGVkID0gZmFsc2U7CiAgICAgV2ViQ29yZTo6 RnJhbWUqIGZyYW1lID0gcGFnZS0+Zm9jdXNDb250cm9sbGVyKCktPmZvY3VzZWRPck1haW5GcmFt ZSgpOwogICAgIFdlYkNvcmU6OkVkaXRvciogZWRpdG9yID0gZnJhbWUtPmVkaXRvcigpOwotICAg IC8vIHdlIGZvcndhcmQgdGhlIGtleSBldmVudCB0byBXZWJDb3JlIGZpcnN0IHRvIGhhbmRsZSBw b3RlbnRpYWwgRE9NCi0gICAgLy8gZGVmaW5lZCBldmVudCBoYW5kbGVycyBhbmQgbGF0ZXIgb24g ZW5kIHVwIGluIEVkaXRvckNsaWVudFF0OjpoYW5kbGVLZXlib2FyZEV2ZW50Ci0gICAgLy8gdG8g dHJpZ2dlciBlZGl0b3IgY29tbWFuZHMgdmlhIHRyaWdnZXJBY3Rpb24oKS4KLSAgICBpZiAoIWhh bmRsZWQpCi0gICAgICAgIGhhbmRsZWQgPSBmcmFtZS0+ZXZlbnRIYW5kbGVyKCktPmtleUV2ZW50 KGV2KTsKKyAgICAvLyBLZXlFdmVudHMgYXJlIHByb2Nlc3NlZCBpbiBzaG9ydGN1dE92ZXJyaWRl RXZlbnQgdG8gY2F0Y2ggdGhlCisgICAgLy8gY2FzZSB3aGVyZSBhIHdlYnBhZ2UgbWlnaHQgaGF2 ZSBhIHNob3J0Y3V0IHN1Y2ggYXMgY3RybCtzIGFuZAorICAgIC8vIHdlIHdhbnQgdGhlIGRvbSB0 byBoYW5kbGUgaXQgYmVmb3JlIGEgUVNob3J0Y3V0IGVhdHMgaXQuCiAgICAgaWYgKCFoYW5kbGVk KSB7CiAgICAgICAgIGhhbmRsZWQgPSB0cnVlOwogICAgICAgICBRRm9udCBkZWZhdWx0Rm9udDsK QEAgLTk4Myw2ICs5ODEsMTQgQEAgdm9pZCBRV2ViUGFnZVByaXZhdGU6OnNob3J0Y3V0T3ZlcnJp ZGVFdmVudChRS2V5RXZlbnQqIGV2ZW50KQogewogICAgIFdlYkNvcmU6OkZyYW1lKiBmcmFtZSA9 IHBhZ2UtPmZvY3VzQ29udHJvbGxlcigpLT5mb2N1c2VkT3JNYWluRnJhbWUoKTsKICAgICBXZWJD b3JlOjpFZGl0b3IqIGVkaXRvciA9IGZyYW1lLT5lZGl0b3IoKTsKKyAgICAvLyB3ZSBmb3J3YXJk IHRoZSBrZXkgZXZlbnQgdG8gV2ViQ29yZSBmaXJzdCB0byBoYW5kbGUgcG90ZW50aWFsIERPTQor ICAgIC8vIGRlZmluZWQgZXZlbnQgaGFuZGxlcnMgYW5kIGxhdGVyIG9uIGVuZCB1cCBpbiBFZGl0 b3JDbGllbnRRdDo6aGFuZGxlS2V5Ym9hcmRFdmVudAorICAgIC8vIHRvIHRyaWdnZXIgZWRpdG9y IGNvbW1hbmRzIHZpYSB0cmlnZ2VyQWN0aW9uKCkuCisgICAgaWYgKGZyYW1lLT5ldmVudEhhbmRs ZXIoKS0+a2V5RXZlbnQoZXZlbnQpKSB7CisgICAgICAgIGV2ZW50LT5hY2NlcHQoKTsKKyAgICAg ICAgcmV0dXJuOworICAgIH0KKwogICAgIGlmIChlZGl0b3ItPmNhbkVkaXQoKSkgewogICAgICAg ICBpZiAoZXZlbnQtPm1vZGlmaWVycygpID09IFF0OjpOb01vZGlmaWVyCiAgICAgICAgICAgICB8 fCBldmVudC0+bW9kaWZpZXJzKCkgPT0gUXQ6OlNoaWZ0TW9kaWZpZXIKZGlmZiAtLWdpdCBhL1dl YktpdC9xdC9DaGFuZ2VMb2cgYi9XZWJLaXQvcXQvQ2hhbmdlTG9nCmluZGV4IDZiMDViMTguLjUx NTRlZWUgMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L3F0L0No YW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDA5LTA4LTExICBCZW5qYW1pbiBDIE1leWVyICA8 YmVuamFtaW4ubWV5ZXJAdG9yY2htb2JpbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFBhc3Mga2V5IGV2ZW50cyB0byB0aGUgZG9tIGluIHNo b3J0Y3V0T3ZlcnJpZGVFdmVudCBzbyB3ZWJzaXRlcworICAgICAgICBsaWtlIEdvb2dsZURvY3Mg Y2FuIGNhdGNoIEN0cmwrUyBiZWZvcmUgYXBwbGljYXRpb24gUVNob3J0Y3V0cyBkby4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI4MTc5CisKKyAgICAg ICAgKiBBcGkvcXdlYnBhZ2UuY3BwOgorICAgICAgICAoUVdlYlBhZ2VQcml2YXRlOjprZXlQcmVz c0V2ZW50KToKKyAgICAgICAgKFFXZWJQYWdlUHJpdmF0ZTo6c2hvcnRjdXRPdmVycmlkZUV2ZW50 KToKKwogMjAwOS0wOC0xMCAgS2F2aW5kcmEgUGFsYXJhamEgIDxrYXZpbmRyYS5wYWxhcmFqYUBu b2tpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgU2ltb24gSGF1c21hbm4uCi0tIAoxLjYu MC40Cgo=