ClementGre / PDF4Teachers

PDF editing software in large quantities. Designed for teachers, this app keeps recorded previous annotations, and offers features like marking scale, PDF conversion, vectorial drawing...
https://pdf4teachers.org/
Apache License 2.0
136 stars 17 forks source link

Fix the bug that the computer language is not 'en' 'fr' or 'it' #178

Closed Clinale closed 6 months ago

Clinale commented 6 months ago

I have identified the problem and it is indeed a bug.

When user first installs and runs PDF4Teachers, and user's computer language is not en fr and it, the PDF4Teachers will run a language selection window for the user to select the target language, and its execution path is as follows:

Main.Main ->Main.setup ->Main.languageAsk ->LanguageWindow.showLanguageWindow ->LanguagesUpdater.update -> LanguagesUpdater.complete ->LanguageWindow.LanguageWindow ->callBack(selectedLanguage)

The problem arises in LanguageWindow.LanguageWindow function, the code for this function is as follows:

public LanguageWindow(CallBackArg<String> callBack){
        super(new ListView<>(), StageWidth.NORMAL, TR.tr("language.chooseLanguageWindow.title"), TR.tr("language.chooseLanguageWindow.title"));
        this.callBack = callBack;

        if(Main.settings.language.getValue().isEmpty()) Main.settings.language.setValue("en_us");
    }

In LanguageWindow, the value of Main.settings.language will be checked, and if the value is empty, it will be set to en_us Afterwards, the callBack (selectedLanguage) is excuted, and the callBack (selectedLanguage) function code is as follows:

new LanguageWindow(selectedLanguage -> {
                if(!selectedLanguage.isEmpty() && !selectedLanguage.equals(Main.settings.language.getValue())){
                    String oldDocPath = TR.getDocFile().getAbsolutePath();

                    Main.settings.language.setValue(selectedLanguage);
                    Main.settings.saveSettings();

                    if(!firstStartBehaviour){
                        Main.window.restart(true, oldDocPath);
                    }else{
                        TR.updateLocale();
                        Main.startMainWindowAuto();
                    }
                }
            });

If the language chosen by the user is fr or it, the codes in the if-condition can be executed normally. However, if the language chosen by the user is en, the condition selectedLanguage.equals (Main.settings.language.getValue ()) is True, causing the if-statement cannot be executed. However, there is no corresponding code to handle this situation, resulting in PDF4Teachers exceptions.

In summary, the condition for this bug to occur is that the user's computer language is not en fr and it, and selecting en_us as the target language when running PDF4Teacher for the first time. Selecting fr and it as the target language will not cause the bug to appear.

The solution is to remove the code that sets Main.settings.language in LanguageWindow.LanguageWindow function. The following is modified codes:

public LanguageWindow(CallBackArg<String> callBack){
        super(new ListView<>(), StageWidth.NORMAL, TR.tr("language.chooseLanguageWindow.title"), TR.tr("language.chooseLanguageWindow.title"));
        this.callBack = callBack;
    }

Originally posted by @Clinale in https://github.com/ClementGre/PDF4Teachers/issues/177#issuecomment-1864076429

ClementGre commented 6 months ago

Thank you very much for this pull request.

I am not sure wether if removing if(Main.settings.language.getValue().isEmpty()) Main.settings.language.setValue("en_us"); is the right thing to do as the language in settings would remain unset.

I instead added a boolean firstStartBehaviour to LanguageWindow, moved the Main.startMainWindowAuto(); out of the if condition in the LanguageWindow callback, added a call to the callback when the user closes the window without clicking on a scene's button, and removing the close button when firstStartBehaviour is to true.

This should fix the bug by making sure that the app is always started after the LanguageWindow is closed. Thank you for finding the reason of this bug!

Clinale commented 6 months ago

In fact, removing if(Main.settings.language.getValue().isEmpty()) Main.settings.language.setValue("en_us"); will not affect settings function and can fix the bug at the minimum cost.

The LanguageWindow.LanguageWindow function is referenced in two places. One is when the user first runs the PDF4Teachers, located in the Main.languageAsk function:

public boolean languageAsk(){
        if(settings.language.getValue().isEmpty()){
            ...
            }else{
                LanguageWindow.showLanguageWindow(true);
                return false;
            }
        }
        return true;
    }

In this function, the argument firstStartBehaviour of the function LanguageWindow.showLanguageWindow is set to true. In this case, the Main.settings.language is empty and the user needs to manually select the target language, which has been discussed in issue #177

The other referencing LanguageWindow.LanguageWindow function is located in Settings.Settings:

public Settings(){
        loadSettings();
        ...
        language.setGetEditPaneCallback(() -> {
            Button button = new Button(TR.tr("actions.choose"));
            button.setOnAction((e) -> LanguageWindow.showLanguageWindow(false));
            button.setDefaultButton(true);
            return new HBox(button);
        });
    }

In this function, the argument firstStartBehaviour of the function LanguageWindow.showLanguageWindow is set to false. In this case, the user has been into the main window of PDF4Teacher, which means the value of Main.settings.language must cannot be empty. Because if the value of Main.settings.language is empty, the users must select the target language in the language window before he/she can enter the main window.

Therefore, removing if(Main.settings.language.getValue().isEmpty()) Main.settings.language.setValue("en_us"); will not affect the functions of PDF4Teachers.

ClementGre commented 6 months ago

Yes, removing Main.settings.language.setValue("en_us"); will not affect the functions of PDF4Teachers as the TR class is using the en_us language as default. The purpose of setting the language before opening the language window is to make its listview select the english language by default.

Even if your fix would resolve your issue, there was another issue that the callback was not always called after closing the LanguageWindow if it was not closed using the Apply button. I changed that by making sure that the callback is always called. Then, I just make sure thatMain.startMainWindowAuto(); is always called from the callback function when it is the first start.

It works as well as your solution, and fixes "another" bug at the same time.