Sigil-Ebook / Sigil

Sigil is a multi-platform EPUB ebook editor
GNU General Public License v3.0
5.73k stars 570 forks source link

[Bug]: Cursor (Save Menu) not blinking! #762

Open hayaku2 opened 3 days ago

hayaku2 commented 3 days ago

Bug Description

When selecting "Save As", or even just Saving for the first time, a Window pops up, with the working file name highlighted in blue, text set in white [SEE IMAGE].

In a large number of cases, the blinking cursor simply does not appear. One can click at various places within the file name, thus changing where text will appear, but the blinking cursor simply isn't there. Makes things very confusing.

This requires closing the "Save As" menu and re-launching, after which, the cursor returns.

IN ORDER TO REPLICATE:

It seems that when you type some new text, there is a short lag, and then the "Preview" window on the right (or wherever it's docked) refreshes. After a "refresh" this bug will activate with 100% probability.

Menu

Platform (OS)

Windows (Default)

OS Version / Specifics

Windows 10

What version of Sigil are you using?

2.2.1

Any backtraces or crash reports

No response

dougmassay commented 3 days ago

It has to be a Qt/Windows thing. To my knowledge, we don't diddle the native Windows Save As dialog at all.

That said... I can't reproduce this on Windows 10. I type some text; wait for Preview to refresh; select Save As from the menu and get a blinking cursor. No matter where I try placing the cursor in the filename.

I've tried with a Light Theme, a Dark Theme, new epub, existing epub, and I always get a blinking cursor in the Save As dialog.

dougmassay commented 3 days ago

You could try disabling QtWebEngine's hardware rendering in Sigil's preferences. If that makes a difference, it could be your video card/drivers. That would be a weird manifestation, but I've seen weirder.

kevinhendricks commented 3 days ago

I can not reproduce this on MacOS either, but MacOS uses non-native file dialogs.

kevinhendricks commented 3 days ago

Since this is a native FileDialog issue, perhaps this is related to a System or FileSystem OS extension installed by the user or computer manufacturer (typically these show file preview images) that is colliding with Qt somehow. Does the issue happen when the system is rebooted into Safe Mode?

hayaku2 commented 2 days ago

Hey guys,

Just tested it on another Windows 10 PC that I have (Sigil 2.02) and same bug on that machine too.

I booted in safe mode, but the bug persists in safe mode as well.

A little more info:

Selecting the "Save as type" (and thus getting the blue select/white text highlight to shift) and then re-selecting the file name solves the issue, causing the cursor to re-appear, for that menu/pop-up instance.

Also discovered that in Safe Mode, preview window refuses to load, but still characteristically 'flashes' a second after new text is entered (it tries to refresh anyway). I noticed in safe mode that typing some text, then rapidly hitting Ctl+Shift+S before the preview 'refresh' can complete, causes the "File name" field to come up without being highlighted/selected in blue, an in this case (and this case only!) the cursor does appear.

I have thus concluded:

It has something to do with the blue highlight/white text feature of Windows, and that the "Preview" refresh may be involved.

Thanks for the help so far! Hopefully this narrows things down...

hayaku2 commented 2 days ago

OK! ANOTHER UPDATE!

Bug does not happen when you click "Save As" from the menu. It has to be Hotkey-triggered, IE Ctl + Shift + S (Save As...) or Ctl + S (Save) for a new file that has not yet been saved/named.

I did a bit more bug testing...

Opening a brand new Sigil file, hitting Ctl + S, bug does not activate.

However, causing the preview pane to refresh even once, does activate the bug, on both Ctl + S (Save) as well as Ctl + Shift + S (Save As...).

Can somebody try to replicate this, using hotkeys and not menu?

dougmassay commented 2 days ago

@kevinhendricks : I suspect this is another case of QtWebEngine stealing focus, and keyboard hotkeys are not sufficient to take the focus back (like clicking the File menu does).

Can confirm the OP's behavior on Windows using keyboard shortcut. No similar behavior on Linux.

kevinhendricks commented 2 days ago

What worries me is that this is a Native Windows FileDialog with only a thin Qt veneer over it. This does not give us much room to play.

As Preview needs and uses Focus we can not assume a destination for last focus to return it to. So this would be an issue. Maybe delaying Save and Save as until the Preview Load is complete before invoking things might work. Just not sure.

dougmassay commented 2 days ago

That's only a guess on my part. The previous bug with QWebEngineView involves a second browser to view the images. So this could still be something different. But the fact that it only happens when using keyboard shortcuts (to the same functions) suggests a focus issue to me.

kevinhendricks commented 2 days ago

Agreed and found this:

https://stackoverflow.com/questions/36609489/how-to-prevent-qwebengineview-to-grab-focus-on-sethtml-and-load-calls

So we may be able to turn off the auto focus grab using the WebProfileMgr which sets the settings. We can try that approach and see what impacts it has.

https://github.com/Sigil-Ebook/Sigil/blob/master/src/Misc/WebProfileMgr.cpp

See the default Initial Settings where we currently enable Focus on Navigation to true. We can test what making this false does.

kevinhendricks commented 2 days ago

So when you get a free moment, please test that change on Windows.

I will be away from a computer for most of today but will test things out when I get back to look for side-effects.

dougmassay commented 2 days ago

Shift+Tab followed by Tab is enough to get things back to normal (highlighted file name with blinking cursor). Is it possible our focus/taborder changes have affected this on Windows only? I'll try building a previous Sigil version (with the same current Qt) and see if the behavior is any different.

dougmassay commented 2 days ago

We cross-posted. I'll test that setting. It could potentially remedy a lot of issues on Windows. There's a lot of double-clutching of Preview at various times. It actually has me wondering about my long-standing issue where dockwidgets that are tabbified with Preview mysteriously change the active tab from time to time.

kevinhendricks commented 2 days ago

It is not the focus on navigation enabled as later in the WebProfileMgr specific to Preview we already set that to false.

So it may be focus highlighting. But if it still happens in Sigil 2.0.2 as reported on the second Windows box, then it can't be that either.

I am stumped.

dougmassay commented 2 days ago

This one could have been around a long time, really.

I just realized that I'm still tooled up with Qt6.6.2 on my Windows 10 dev machine. I'll try a little later with Qt6.7.2 and see if anything changes.

On Fri, Jul 5, 2024, 8:24 AM Kevin Hendricks @.***> wrote:

It is not the focus on navigation enabled as later in the WebProfileMgr specific to Preview we already set that to false.

So it may be focus highlighting. But if it still happens in Sigil 2.0.2 as reported on the second Windows box, then it can't be that either.

I am stumped.

— Reply to this email directly, view it on GitHub https://github.com/Sigil-Ebook/Sigil/issues/762#issuecomment-2210785130, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACG3CXRX5GQUS2YTIQZVV6DZK2GATAVCNFSM6AAAAABKLSZ6QWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJQG44DKMJTGA . You are receiving this because you commented.Message ID: @.***>

dougmassay commented 2 days ago

I've gone as far back as Sigil 1.8.0 (with Qt5.12.9) and this issue is still present. So it's nothing new. It's either a longstanding Qt bug that's never been addressed, or it's something we're doing. But as you say, with native dialogs on Windows, there's not much we're able to affect.

dougmassay commented 2 days ago

And it's still an issue when using Qt6.7.2

kevinhendricks commented 2 days ago

One thing to play with might be changing the delay after typing ends to when the reload happens to be either longer or shorter. In 2.2.1 there is an environment variable that can be used to set this once at first Sigil startup. It is 1000 ms now but can go as low as 100 ms or as high as 10000 ms.

If the issue is Preview Loading (or reloading after a text change) it would be interesting to see if we can impact this by timing. It it still happens as before, then the issue may not directly be Preview reloading at all.

dougmassay commented 2 days ago

I don't think it's timing issue with the Preview refresh. I tried a lot of different refresh timeouts with no change (regardless if I waited for the refresh or hit Ctrl+Shift+S before the refresh happened).

I think we might be able to rule out Preview entirely. The same thing happens when Preview is closed. In fact, the modifying of the file in Code View is unnecessary. All you have to do is change the default cursor position after opening a file in Code View and the issue manifests (when using the Ctrl+Shift+S shortcut).

After moving the cursor, I can switch focus from Code View to any other dock widget before hitting Ctrl+Shift+S and the problem does not manifest.

I'm not sure if that helps or just adds more noise. :)

kevinhendricks commented 2 days ago

This is one strange bug. So with Preview closed just clicking anyplace else in CodeView before hitting Ctrl+Shift+S is enough to cause the issue? What about using the arrow key to move the just a space or two the right. Is something that small enough to cause the issue.

Even though Preview is closed, every time the cursor is moved by pointing and clicking CodeView sends out a signal that tells Preview to sync itself to their new position. Even if Preview is closed that signal goes out.

Very strange. And why this would have an impact on only Windows is so strange.

One thing else to test, try changing the ifdef tests around the Save or Save-As Filedialogs to temporarily use the non-native file dialog (just like MacOS does). If the problem still happens we can at least track down where in the non-native code the issue is happening.

kevinhendricks commented 2 days ago

I think just commenting out this #if and its #endif in MainWindow.cpp in SaveAs() should be enough to force use of Qt FileDialogs.

https://github.com/Sigil-Ebook/Sigil/blob/master/src/MainUI/MainWindow.cpp#L1962

Then Windows will use NonNative filedialogs just like Linux and MacOS do.

Worth a shot?

kevinhendricks commented 2 days ago

And could Windows already have defined that Ctrl+Shift+S to do something else internally. Have you tried using Sigil's Preferences to use some shortcut key sequence maybe not starting with Ctrl? Perhaps that is a Windows standard Save or Save as hotkey sequence?

That might be worth a shot to rule that hypothesis out.

dougmassay commented 2 days ago

The easy ones first. :)

What about using the arrow key to move the just a space or two the right. Is something that small enough to cause the issue.

Yes that still causes the issue.

And could Windows already have defined that Ctrl+Shift+S to do something else internally. Have you tried using Sigil's Preferences to use some shortcut key sequence maybe not starting with Ctrl? Perhaps that is a Windows standard Save or Save as hotkey sequence?

The issue seems to follow the shortcut regardless of the combinations (many starting with something other than Ctrl). But interestingly enough, I discovered that the Ctrl+Shift combo toggles different Windows keyboard languages (if multiple are installed). But it doesn't seem to interfere with 3 key Ctrl+Shift+? Combos so long as you don't release before the 3rd key. I couldn't figure out why I was getting incorrect characters when typing!

I'll try compiling with non-native dialogs and see if it makes a difference. I can't remember if there was a reason we were hanging on to native on Windows.

kevinhendricks commented 2 days ago

Native file dialogs are supposed to be better but they do not really exist for some Linux boxes and on macOS they were constantly crashing! That is the only reason macOS uses Qt ones as far as I know. I could never track down the crashes but that was quite a while back. I finally gave up and moved away from the native ones.

Many people have complained about the non-natives ones on macOS but it can do everything the native one can do and does not crash. Furthermore Sigil is not just a macOS app, it is a cross-platform app so I really did not give a damn what the FileDialogs look like, as far as they work correctly and did not crash!

kevinhendricks commented 2 days ago

I thought I remembered a similar issue with a QLineEdit way back in the Sigil 0.9.13 time period. I found this commit that worked around the bug:

https://github.com/Sigil-Ebook/Sigil/commit/a8acab7e375d29837bebe330c1d2b8c8ee07ca28

If you read the comment, the reason the QLineEdit was acting that way was that it was waiting for a Key Release event of some sort. We faked it by simulating a shift key press and release.

Once the issue happens in the native filedialog, will hitting and releasing the shift key "fix" things?

kevinhendricks commented 2 days ago

If hitting shift helps, my guess is Qt launches its shortcuts with a keypress and the key release is lost by Windows, or visa versa. Sort of a leading vs trailing edge issue.

This would explain why this is happening only with shortcuts that cross over to native. Or Windows is consuming one or them in their own short cut processing.

dougmassay commented 2 days ago

Hitting the shift key had no effect on the situation.

Going non-native does seem to eliminate the problem. Using non-native dialogs wouldn't bother me at all, but I fear it would be quite the learning curve for many Windows users. They'd have to add a lot of stuff to the left pane that is typically always "just there" on Windows. There's also the lost functionality of being able to expand/collapse folders/drives in the left pane.

If Windows had crashing issues with native dialogs, I'd jump on Qt dialogs in a heartbeat. But I wouldn't relish having to deal with the outpouring of complaints from Windows users over the change. Especially for a fairly innocuous bug that has been around for years with no complaints until this one.

Perhaps an opt-in solution for non-native dialogs on Windows for users who just can't live with a missing cursor when calling SaveAs with a keyboard shortcut and wanting to do granular edits to the new filename? :)

Though non-native DOES have the advantage of honoring Sigil's new double-width cursor setting. ;)

kevinhendricks commented 2 days ago

It is your call. You are right, It is a very minor issue at best. It still might be worth a QtBug search to see if anyone else has reported anything similar.

And I do wonder if our own calls to QApp processEvents() are part of the problem so that some of the events are not being processed earlier or are processed out of order. I wonder if all of them need an ExcludeUserInputEvents added.

Or maybe if we add a call to processEvents() to the start of the SaveAs() routine in MainWindow.cpp would it help prevent these issues. It might be worth checking.

Other than that I am out of ideas.

kevinhendricks commented 2 days ago

Here is the only QtBug that might be related:

https://bugreports.qt.io/browse/QTBUG-109065?jql=project%20in%20(QTBUG%2C%20PYSIDE)%20AND%20issuetype%20%3D%20Bug%20AND%20text%20~%20%22qfiledialog%22%20ORDER%20BY%20key%20DESC%2C%20component%20ASC