JeffHoogland / ePad

A simple text editor written in python and elementary
GNU General Public License v3.0
4 stars 8 forks source link

unsavedWorkPopup segfault #17

Closed rbtylee closed 7 years ago

rbtylee commented 7 years ago

In cases where the user has selected Use Elementary icons in elementary_config, epad segfaults trying to load the unsavedWorkPopup. This pop up is displayed when the user Xes out ePad with unsaved work.

This is an issue with users updating from past versions to efl.19.1 because the default configuration for efl has changed to setting Use Elementary Icons in efl 19.1. So for users that have never set anything in elementary_config, efl nows defaults to using elm theme icons. This is a change from old versions of efl. Users that have user elm configuration settings naturally the old settings are unchanged in the update.

Segmentation fault is the only terminal error message displayed in these cases.

rbtylee commented 7 years ago

For reference this is a manifestation of an icon issue, that has been noted before.

Specifically the code:

        try:
            dialogImage = Image(self.confirmPopup,
                                size_hint_weight=EXPAND_HORIZ,
                                size_hint_align=FILL_BOTH,
                                file=icon.file_get())
            tb.pack(dialogImage, 0, 0, 1, 1)
            dialogImage.show()
        except RuntimeError:
            # An error message is displayed for this same error
            #   when aboutWin is initialized so no need to redisplay.
            pass
kaihu commented 7 years ago

Wait, why is it segfaulting then?

kaihu commented 7 years ago

Okay, for the record it's segfaulting because icon file is (None, 'strict') when elm_config.icon_theme is ELM_CONFIG_ICON_THEME_ELEMENTARY

rbtylee commented 7 years ago

Yep I had already noticed that. From a python-efl point of view perhaps a segfault is not a good way of handling that error. Perhaps py-efl can be modified to throw a python exception or something in cases like this?

kaihu commented 7 years ago

Yeah I can check for None in path and error out of it more gracefully. Although the C api shouldn't be crashing when we pass NULL there...

rbtylee commented 7 years ago

I wasn't aware that the c API was also crashing in this situation :( Have you filed a phab issue on this?

kaihu commented 7 years ago

https://phab.enlightenment.org/T5697

rbtylee commented 7 years ago

@kaihu, you must have magic superpowers to get this issue closed so fast :+1:

Anyway, I modified epad to use Icon directly instead of converting to an image. Simpler code better results. I am unsure why we did it that way to begin with, must have had an issue that forced us to that is now resolved in EFL. Another reminder to myself to always report weird efl issue to someone or to phab.