Project

General

Profile

Actions

Defect #36932

closed

Handle nil return of Redmine::Themes.theme(Setting.ui_theme) in Redmine::Info.environment

Added by Mischa The Evil over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Administration
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

The code introduced in r21308 for #32116 can trigger an undefined method error in some edge-case(s). See Admininistration>Information page Internal error.
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

Related to Redmine - Feature #32116: Add configured theme to Redmine::InfoClosedGo MAEDA

Actions
Actions #1

Updated by Mischa The Evil over 2 years ago

  • Related to Feature #32116: Add configured theme to Redmine::Info added
Actions #2

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.

Actions #3

Updated by Marius BĂLTEANU over 2 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF