Bug 768704

Summary: improve shortcut matching code
Product: [Fedora] Fedora Reporter: Matthias Clasen <mclasen>
Component: libxfce4uiAssignee: Christoph Wickert <cwickert>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: cwickert, kevin, maxamillion
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libxfce4ui-4.8.1-1.fc15 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-12-31 20:25:15 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 759478    
Bug Blocks:    
Attachments:
Description Flags
patch none

Description Matthias Clasen 2011-12-18 05:37:21 UTC
Created attachment 548339 [details]
patch

A change in gtk_accelerator_name in gtk 2.24.8 exposed a weakness in the shortcuts handling in xfce. The code in xfce-shortcuts-grabber.c assumes that it is safe to compare string representations as a way to match shortcuts. This is problematic, since there is no 1-1 mapping between string representations and keyboard shortcuts - consider e.g. <Ctrl><Alt>Del vs <Alt><Ctrl>Del. One way to make this a little more robust is to always roundtrip both strings through shortcuts_parse+shortcuts_name - that way, you are at least always comparing strings that have been produced by the same algorithm. This is what the attached patch does. It fixes the <Control> vs <Primary> problem.

Comment 1 Christoph Wickert 2011-12-18 10:19:10 UTC
Thanks for the patch. It makes the old shortcuts work again, but it doesn't really fix the <Control> vs <Primary> problem.

A single key is treated as <Control> now, but as soon as a second one gets pressed it becomes <Primary> again. Unfortunately nothing responds to <Primary> any longer, so newly created shortcuts will not work.

Not sure if the problem is in xfce4-keyboard-settings, in xfce4-settings-manager or in libxfce4ui.

Comment 2 Matthias Clasen 2011-12-19 11:49:39 UTC
> A single key is treated as <Control> now, but as soon as a second one gets
> pressed it becomes <Primary> again.

What does that mean, where does it 'become <Primary> again' ?
Can you give me a concrete example of a shortcut that does not work ?

Comment 3 Christoph Wickert 2011-12-19 11:54:44 UTC
(In reply to comment #2)
> > A single key is treated as <Control> now, but as soon as a second one gets
> > pressed it becomes <Primary> again.
> 
> What does that mean, where does it 'become <Primary> again' ?

When you add a new shortcut in xfce4-keyboard-settings, you first enter the command and then press the desired keys. When you press Ctrl, it correctly displays <Control_L> or <Control_R>, but as soon as you press a second or third key, it displays <Primary>.

> Can you give me a concrete example of a shortcut that does not work ?

All newly created shortcuts, you can try whatever you want. It's no longer possible to add one. This worked before the patch, it was <Primary> all the time.

Comment 4 Matthias Clasen 2011-12-19 13:06:55 UTC
It displays <Primary> because the xfce4-keyboard-settings code displays the internal accelerator representation that is returned by gtk_accelerator_name(), instead of the human-readable string that is returned by gtk_accelerator_get_label().

I'll investigate what goes wrong with additions

Comment 5 Matthias Clasen 2011-12-19 13:25:39 UTC
I just tried two things:

1) I edited the <Control>Escape shortcut, and replaced it by the same - that turns the displayed string into <Primary>Escape. But the shortcut continues to work as expected.

2) I added a new <Control><Shift>c shortcut and made it start gcalctool. It is displayed as <Primary><Shift>c, but it works just fine.

Comment 6 Christoph Wickert 2011-12-19 13:41:04 UTC
I think you are right and I tested wrong. Thanks a lot for all your efforts!

Comment 7 Fedora Update System 2011-12-19 17:59:30 UTC
libxfce4ui-4.8.0-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/libxfce4ui-4.8.0-6.fc16

Comment 8 Fedora Update System 2011-12-19 17:59:55 UTC
libxfce4ui-4.8.0-6.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/libxfce4ui-4.8.0-6.fc15

Comment 9 Fedora Update System 2011-12-22 22:36:11 UTC
Package libxfce4ui-4.8.0-6.fc16:
* should fix your issue,
* was pushed to the Fedora 16 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing libxfce4ui-4.8.0-6.fc16'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2011-17273/libxfce4ui-4.8.0-6.fc16
then log in and leave karma (feedback).

Comment 10 Fedora Update System 2011-12-24 12:10:08 UTC
libxfce4ui-4.8.1-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/libxfce4ui-4.8.1-1.fc15

Comment 11 Fedora Update System 2011-12-31 20:25:15 UTC
libxfce4ui-4.8.1-1.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 12 Fedora Update System 2012-01-05 20:58:41 UTC
libxfce4ui-4.8.1-1.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.