vslavik / poedit

Translations editor for Mac, Windows and Unix
https://poedit.net
MIT License
1.77k stars 275 forks source link

Incorrect rendering of issue status icon with wxGTK 3.1.6 #746

Closed super7ramp closed 2 years ago

super7ramp commented 2 years ago

Environment

Bug

The icons indicating the presence of an issue or a comment on the translatable strings are incorrectly updated when scrolling up/down the list of strings.

It looks like the strings without issue/comment "reuse" the icons of other visible strings with an issue or a comment.

Here's a screencast showing the current behaviour (3.1.6):

https://user-images.githubusercontent.com/9394745/163816668-77f1c3c4-0860-482a-8370-5241dd539976.mp4

Here's a screencast showing the expected behaviour (3.1.5):

https://user-images.githubusercontent.com/9394745/163816494-01653885-1d0a-442a-8507-6b89020ea8ab.mp4

I suspect this a wxWidgets-only problem: Bisected the issue between wxWidgets 3.1.5 and 3.1.6 and found that commit: e067e42, i.e. problem is not visible anymore with a wxWidget 3.1.6 patched to exclude this commit.

So this ticket may seem irrelevant but I'd like to confirm there's nothing to be done on Poedit's side here - I'll report this issue to wxWidgets then.

Steps to reproduce

  1. Decompress and open the following translation file: zypper-master-fr.po.gz
  2. (Optional) In order to get the same string ordering as in the screencasts above, configure View with the following:
    • Show warnings
    • Sort by file order
    • Group by context
    • Entries with error first
    • Untranslated entries first
  3. Scroll down the string list
vslavik commented 2 years ago

So this ticket may seem irrelevant but I'd like to confirm there's nothing to be done on Poedit's side here - I'll report this issue to wxWidgets then.

I don't know what to make of it - the relevant things I was involved in in wx happened in 2015/16 timeframe and my memory isn't that good. This certainly can be worked around in Poedit (by using a "fake" transparent bitmap) if needed, but I am not entirely sure this change by @vadz in wx-3.1.6 was intentional.

There's history of commits that indicate it was desirable to allow NULL values for bitmap renderers. Commit messages of https://github.com/wxWidgets/wxWidgets/commit/f3b8dac3b7858d7d020daeda5421a63accd83435, https://github.com/wxWidgets/wxWidgets/commit/8ad375592c94e8077adf8bc1e5d00d8960885816 and https://github.com/wxWidgets/wxWidgets/commit/ae93a83e76e99a0d089fd8e1d5b027f14bfdaaa3, where I contributed fixes in this handling, indicate that some wx ports already did it like that and it appears I thought this was sufficiently clear-cut that I didn't ask @vadz for a review...

vadz commented 2 years ago

The problem is that my memory is also very poor and so I didn't trust it when discussing this problem in https://github.com/wxWidgets/wxWidgets/issues/18934 (and maybe other issues, I think there was more than one). I.e. I thought that we needed the original fix but couldn't remember why exactly and when I couldn't reproduce the problem without it, I've reverted it because it did create other problems (as mentioned in that ticket).

Anyhow, we'll have to re-revert (un-revert? de-revert? vert?) it again and fix these problems in some other way, but could you please point me to the code allowing to reproduce the problem if you already know where it is? I am not sure if we can add an automatic test for this, but I'd like to at least have a manual test that could be used to check whether this works correctly.

And it's probably better to migrate this issue to wxWidgets, if possible. TIA!

super7ramp commented 2 years ago

Thank you both @vslavik and @vadz for your comments.

I've copied the ticket to wxWidgets/wxWidgets#22359. I added a pointer on where I believed the code relative to the icon column was. I haven't created a small reproducer yet - my knowledge of wxWidgets and C++ is very low to say the least so it may take a while.

I'm closing this ticket now.

vslavik commented 2 years ago

I haven't created a small reproducer yet

I'll do it.

Reopening for possible Poedit part of the fix (at the very least, probably requiring wxGTK >= 3.1.7).

vslavik commented 2 years ago

…and re-closing because the fix was entirely in wx, nothing left to do here.