Defect #36932
closedHandle nil return of Redmine::Themes.theme(Setting.ui_theme) in Redmine::Info.environment
0%
Description
The code introduced in r21308 for #32116 can trigger an undefined method error in some edge-case(s). See .
It turns out that it can happen that Setting.ui_theme.blank?
returns false
while Redmine::Themes.theme(Setting.ui_theme)
returns nil
. This is not expected while sending a message to Redmine::Themes.theme(Setting.ui_theme).javascripts.include?
in source:/trunk/lib/redmine/info.rb@21529#L25.
The error can be reproduced (manually) on the console by setting a non-existing value for Setting.ui_theme
like e.g. Setting.ui_theme = 'foo'
and subsequently sending a message to Redmine::Info.environment
. This effectively simulates the edge-case that is reported in the forum thread by Greg G.
This can be solved by changing lines 23-32 in source:/trunk/lib/redmine/info.rb@21529#L23 from:
theme = Setting.ui_theme.blank? ? 'Default' : Setting.ui_theme.capitalize
unless Setting.ui_theme.blank?
theme_js = (if Redmine::Themes.theme(Setting.ui_theme).javascripts.include?('theme')
' (includes JavaScript)'
else
''
end
)
end
theme_string = (theme + theme_js.to_s).to_s
to: theme_string = ''
theme_string += (Setting.ui_theme.blank? ? 'Default' : Setting.ui_theme.capitalize)
unless Setting.ui_theme.blank? ||
Redmine::Themes.theme(Setting.ui_theme).nil? ||
!Redmine::Themes.theme(Setting.ui_theme).javascripts.include?('theme')
theme_string += ' (includes JavaScript)'
end
This change also fixes the issue that the theme_js
variable is defined conditionally but is unconditionally "used", which I feel is bad practice.
This issue affects 5.0.0 and current source:/trunk@21529.
Related issues
Updated by Mischa The Evil over 2 years ago
- Related to Feature #32116: Add configured theme to Redmine::Info added
Updated by Marius BĂLTEANU over 2 years ago
- Status changed from Confirmed to Resolved
- Assignee set to Marius BĂLTEANU
- Resolution set to Fixed
Committed the patch with a test.
Updated by Marius BĂLTEANU over 2 years ago
- Status changed from Resolved to Closed