Fix hotkeys in BitmapView #5626
Conversation
This allows hotkeys to be set for BitmapView windows, such as basic copy and paste. Without including BitmapView hotkeys in share/default, the hotkeys set within this file still don't work. However, none of the views that already had working hotkeys set their hotkeys in their C code, instead leaving them all set to 'No Shortcut'. This fixes bug fontforge#5150 ('No Hotkeys/Shortcuts in BitmapView').
This completes the fix for bug fontforge#5150 by adding the hotkeys set within fontforgeexe/bitmapview.c to share/default.
|
|
||
| bv->gw = gw = GDrawCreateTopWindow(NULL,&pos,bv_e_h,bv,&wattrs); | ||
| free( (unichar_t *) wattrs.icon_title ); | ||
| GDrawSetWindowTypeName(bv->gw, "BitmapView"); |
There was a problem hiding this comment.
It looks like other views also call GDrawSetWindowTypeName() for the drawing subwindow v, like GDrawSetWindowTypeName(cv->v, "CharView");
In Linux the code seems to work without it, but there could theoretically be focus-related issues on other platforms.
There was a problem hiding this comment.
I had initially included both, but after some testing I removed the one going to bv->v since it didn't seem necessary. I'll admit, I haven't tested on any other platforms.. I don't have easy access to a Windows computer right now (haven't booted into Windows in years and I know it'll try to install tons and tons of updates and take all day before I can actually use it), let alone a Windows computer with the necessary libraries and compiling tools necessary.
I can add it in a second commit though anyway; it didn't seem to cause any problems to be there, and if there's the possibility that not having it can cause problems on other platforms then it's probably best to include it.
There was a problem hiding this comment.
Ok, then add the call with bv->v please, just to be on the safe side.
It's possible that only adding the type name for the top-level window could lead to focus problems on other platforms. Other window types where hotkeys work also set this value for their primary subwindow anyway, so it's probably a good idea to include.
iorsh
left a comment
There was a problem hiding this comment.
Looks good to me, tested on my machine (Ubuntu 24.04) and it works.
|
@iorsh I've also considered fixing up the menus' I actually wasn't expecting submenus to work without doing the latter, but they do, so I didn't touch them. And yeah, I know that none of the submenu items have a hotkey assigned to them by default, but I tried it anyway by adding one to my local user-level 'hotkeys' file, and it did actually work. I don't think the code for that would depend on the platform, but I don't know for sure? At any rate, I think I can do that if you think it'd be a good idea to do that at the same time; otherwise it could be a separate pull request. |
|
@iorsh I don't have authorization to push things to master, so it tells me merging is blocked. |
If custom submenu hotkeys already work, we should probably leave the code as is. I think these |
I'll merge it in a few days if there are no further comments from other developers. |
It turns out that in order to get BitmapView's hotkeys working, the top-level window (bv->gw) needs its 'Type Name' set (by calling
GDrawSetWindowTypeName(bv->gw, "BitmapView");after it's created in fontforgeexe/bitmapview.c), and the hotkeys added to share/default.None of the other views (such as CharView, FontView, and MetricsView) even set the hotkeys within the source code itself anymore, and doing so doesn't actually work it seems, so I could also add a commit to set all of those to 'No Shortcut' within the C code.. But in the meantime, I wanted to have the minimal number of changes to the code, so that this fix could be included as easily as possible.
I also noticed that the share/default file is kinda.. Haphazardly organized. It's not clear whether these should be added to the start or the end, since mostly (but not entirely) the views are organized alphabetically (CharView, FontView, MetricsView), but there's some CharView things added to the very top and very bottom, seemingly out of place.
To avoid messing with where existing hotkeys exist within the file I added the new BitmapView hotkeys to the bottom, but I can move them to the top if that's preferred.
Type of change