Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncaught error if 'start_documents_uri' not defined in prefs #279

Closed
bwbohl opened this issue Jul 4, 2022 · 7 comments · Fixed by #439
Closed

Uncaught error if 'start_documents_uri' not defined in prefs #279

bwbohl opened this issue Jul 4, 2022 · 7 comments · Fixed by #439
Assignees
Milestone

Comments

@bwbohl
Copy link
Member

bwbohl commented Jul 4, 2022

app.js:1 Uncaught constructor {msg: 'No preference found with this key', key: 'start_documents_uri', level: 'warn', sourceMethod: 'getPreference', sourceClass: 'EdiromOnline.controller.PreferenceController', …}

@bwbohl bwbohl added this to the prefs, customization milestone Jul 4, 2022
@krHERO krHERO moved this to New in Edirom Development Jul 3, 2024
@krHERO krHERO removed this from the prefs, customization milestone Jul 4, 2024
@bwbohl
Copy link
Member Author

bwbohl commented Sep 6, 2024

This does not result in any malfunction but just doesn’t seem right and bad style.

@bwbohl
Copy link
Member Author

bwbohl commented Sep 12, 2024

Apparently it is sufficient if the prefs file contains an empty entry in the form:

<entry key="start_documents_uri" value=""/>

@daniel-jettka
Copy link
Contributor

So this is where it may happen that the warning is thrown if there is no entry in the prefs file for start_documents_uri:
(1) https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L130
(2) https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L183

What do you think @bwbohl - is there something to fix or a better way to handle things is the entry is not there?

@bwbohl
Copy link
Member Author

bwbohl commented Oct 9, 2024

I think if we add a check (probably in in (2)), wheter the return value of var uris is empty or null and if so return; that would do the job.

@daniel-jettka
Copy link
Contributor

Where and when is the error message thrown anyway? :-)

Can there be an additional parameter here? - https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L185

So that
var uris = me.getController('PreferenceController').getPreference('start_documents_uri');

becomes
var uris = me.getController('PreferenceController').getPreference('start_documents_uri', true);

The definition of getPreference looks like that - https://github.com/Edirom/Edirom-Online/blob/develop/app/controller/PreferenceController.js#L64
getPreference: function(key, lax) { ... }

Would test that if I knew how to throw the error...

@bwbohl
Copy link
Member Author

bwbohl commented Oct 9, 2024

If you lad an edirom online and the edition has not defined a value for key start_documents_uri in its preferences the error is thrown. You located the right spoot in the JS that throws the error.

getPreference: function(key, lax) {
var me = this;
if(!me.preferences[key] && lax)
return null;
if(key == "application_language") {
var lang = me.getURLParameter("lang");
if(lang) {
return lang;
} else {
return "en";
}
}
if(me.preferences[key] == "" && key == "image_prefix") {
return "";
}
if(!me.preferences[key]) {
Ext.Error.raise({
msg: 'No preference found with this key',
key: key,
level: 'warn' //warn, error, fatal
});
return null;
}
return me.preferences[key];
},

to be exact:

if(!me.preferences[key]) {
Ext.Error.raise({
msg: 'No preference found with this key',
key: key,
level: 'warn' //warn, error, fatal
});

The error can be observed in the Developer Tools JS console of your browser.

@bwbohl
Copy link
Member Author

bwbohl commented Oct 9, 2024

This is not a critical issue, as you can see from the error level in line 87 which is set to warn. But I think it’s nicer to catch it before an warning is raised, e.g. by avoiding the execution if the preferences key is an empty string or null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants