MobileRead Forums

MobileRead Forums (https://www.mobileread.com/forums/index.php)
-   Sigil (https://www.mobileread.com/forums/forumdisplay.php?f=203)
-   -   Possible bug? (https://www.mobileread.com/forums/showthread.php?t=341626)

KevinH 09-09-2021 03:50 PM

We can change things to ifdef that Windows routine and convert the nativeVirtualKeyCode on Windows if that is what Windows users want.

I personally expect when entering Meta+Shift+1 to see Meta+! and ditto for Alt, as a result because Shift+1 is the ! character and that is how it typically worked on MacOS and Linux.

But I am open to either since I am not a Windows user and as long as we do not need to change the Sigil link command to include a new library.

BeckyEbook 09-09-2021 03:59 PM

Quote:

Originally Posted by KevinH (Post 4152926)
But I am confused ... did calibre's code work correctly and you just did not "like" the Shortcut shown, or did it actually fail with some key sequences and that is why you did not "like" the Shortcut shown?

Calibre's code work correctly, and newest build Sigil's work correctly too.
I just don't like the shortcut shown, but obviously it's more important to get it right than display the shortcut.

Quote:

Originally Posted by KevinH (Post 4152926)
And what do you mean by "when in the field with the shortcut"?

Fiels = Input "Shortcut" in Preferences > Keyboard Shortcuts.

Quote:

Originally Posted by KevinH (Post 4152926)
Do you mean the "Ctrl+!" output shown? If so, that is not meant to be an input field to be edited. Entering other keys there will create issues. For example the del key or backspace will erase the key sequence completely. It is not meant for editing directly but to display the key sequence entered.

Of course I understand that!

Quote:

Originally Posted by KevinH (Post 4152926)
Or do you mean the table entry itself or when editing the ini?

The entry itself in the table. But when I think about it, the entry in the table is not crucial, because correct operation is more important. So let's admit that it's better, although it is worth having other people testing it with international keyboards.

BeckyEbook 09-09-2021 04:13 PM

2 Attachment(s)
But there is one thing that works worse.
Well – if you remember – in Windows, users using international keyboards should add the -platform windows:altgr parameter, which allows, for example, to easily call even the default keyboard shortcuts for Clips.

Sigil 1.7.0 (with parameter) nicely distinguishes left Alt and right Alt, and unfortunately the latest build displays garbage for right Alt+letter.

KevinH 09-09-2021 04:20 PM

Hmm okay. Does your version using the Windows call to map the nativeVirtualKeyCode to its char work properly with -platform windows:altgr

If so that is a strong argument to go with your added Windows call approach.

Or did we mess that up with an earlier change to master?

DiapDealer 09-09-2021 04:41 PM

If all we're doing is removing the Shift on Windows when the Shift is unnecessary (and problematic), I can't see how that would affect the display of a Right Alt+letter sequence on an International Keyboard. I'm confused.

DiapDealer 09-09-2021 04:56 PM

Quote:

Originally Posted by BeckyEbook (Post 4152934)
But there is one thing that works worse.
Well – if you remember – in Windows, users using international keyboards should add the -platform windows:altgr parameter, which allows, for example, to easily call even the default keyboard shortcuts for Clips.

Is that a command-line parameter for running Sigil? This is the first I've heard of international keyboard users needing to add special parameters for running Sigil. Would most users even know how? If it's necessary, then we should probably find a way to load it programmatically via a pref setting rather than relying on people stumbling across an obscure parameter to the qwindow platform plugin. Can you point me to some documentation on that platform plugin parameter? I can't seem to find anything.

BeckyEbook 09-09-2021 05:02 PM

Quote:

Originally Posted by DiapDealer (Post 4152941)
If all we're doing is removing the Shift on Windows when the Shift is unnecessary (and problematic), I can't see how that would affect the display of a Right Alt+letter sequence on an International Keyboard. I'm confused.

I specially installed an additional Spanish keyboard layout in which AltGr+;=ñ
And now, when in the Keyboard Shortcuts window I select the AltGr+; then I get garbage.
But I note that this only happens when starting Sigil with the parameter.

Again, I suspect this is such an extremely rare situation for another Windows user to encounter this problem that I wonder if it is worth wasting your precious time on it.

It would have to be:
a) Windows user who
b) uses the international keyboard and obtains additional characters using AltGr
c) runs Sigil with an additional parameter
d) would have the crazy idea to use a shortcut with AltGr if he can use the left Alt for this and then everything works fine.

DiapDealer 09-09-2021 05:08 PM

I'm not opposed to looking into it, I'm just completely in the dark on there being an altgr platform plugin parameter in the first place. :)

BeckyEbook 09-09-2021 05:09 PM

Quote:

Originally Posted by DiapDealer (Post 4152944)
Can you point me to some documentation on that platform plugin parameter? I can't seem to find anything.

https://doc.qt.io/qt-5/qguiapplicati...GuiApplication

We talked about it here:
https://github.com/Sigil-Ebook/Sigil/issues/474

the parameter was mentioned e.g. here:
https://bugreports.qt.io/browse/QTBU...comment-504202

https://bugreports.qt.io/browse/QTBU...comment-416209

is also in Sigil's documentation:
https://github.com/Sigil-Ebook/sigil...faq.xhtml#L228

KevinH 09-09-2021 05:44 PM

One other difference, in the calibre way we simply remove the shift modifier and leave all other modifiers alone, but in the old way, we started with 0 and or'd in just the flags we wanted to use. So if there is an altgr flag mapped to a GroupModifier it got lost in the old way. Perhaps that is what we need to do under calibre's approach as well?

Also the two approaches treat number (digits) chars differently based on text() values.


Quote:

Originally Posted by DiapDealer (Post 4152941)
If all we're doing is removing the Shift on Windows when the Shift is unnecessary (and problematic), I can't see how that would affect the display of a Right Alt+letter sequence on an International Keyboard. I'm confused.


KevinH 09-10-2021 09:14 AM

@BeckyEbook, I will need help tracking down what got broken with the altgr command line switch being enabled in order to fix it.

What does the debug output say when using it?

How does the list of modifiers (value of state) differ between when a normal alt is done vs an altgr?

Or are the modifiers unchanged and it just changes the key() or text() value to reflect the fact that altgr us being used?

BeckyEbook 09-10-2021 09:59 AM

2 Attachment(s)
Debug.

The order of my shortcuts:
  • h
  • Shift+h
  • Ctrl+h
  • leftAlt+h
  • AltGr+h
  • 1
  • Shift+1
  • Ctrl+1
  • leftAlt+1
  • AltGr+1
  • leftAlt+l (lowercase L)
  • AltGr+l (ł on Polish keyboard)
  • Shift+AltGr+l (Ł on Polish keyboard)

Issue (garbage visible in "Shortcut" input box) occurs only when the parameter is enabled and only when using the shortcut from AltGr. The other shortcuts appear identical.

KevinH 09-10-2021 10:20 AM

If you get a chance can you copy the qdebug and add it to the working Sigil 1.7.0 version, use the altgr command line with it and run the exact same tests and post its output as well.

That way can see what changes if anything between working and non-working versions when altgr is added to the command line.

At first glance, adding altgr to command line changed the modifier flags from both Ctrl+Alt used to GroupModifier used which seems to match the qt docs but KeyboardShortcuts.cpp handleKey never even looks or adds in the GroupModifier so I am unsure how that worked at all!

BeckyEbook 09-10-2021 10:49 AM

1 Attachment(s)
QDebug added to 1.7.0.

KevinH 09-10-2021 10:55 AM

I searched the Qt 5.12 codebase for GroupSwitchModifier and it is not used in QKeySequence at all (qtbase/src/gui/kernel/qkeysequence.cpp). In fact it appears to not be supported in much code at all.

In Sigil 1.7 in KeyboardShortcutsWidget.cpp we build up the resulting modifier by checking modifier flags but do not handle the GroupSwitchModifier at all so it gets stripped out but the actual key() value reflects its use with altgr in the command line.

In current master, the key() with altgr in the command line does reflect its use but the GroupSwitchModifier is kept. And that seems to confuse QKeySequence since it does not grok the GroupSwitchModifier at all.

I will try in master or Windows checking for the GroupModifier and if set remove it before trying calibre's approach in the hopes everything just works.


All times are GMT -4. The time now is 09:18 PM.

Powered by: vBulletin
Copyright ©2000 - 3.8.5, Jelsoft Enterprises Ltd.
MobileRead.com is a privately owned, operated and funded community.