Opened 12 years ago

Last modified 10 years ago

#171 reopened defect

Clicking the "Reclassify" button can corrupt the database

Reported by: Brian Smith Owned by: Brian Smith
Priority: normal Milestone:
Component: User Interface Version: svn
Severity: normal Keywords: reclassify corpus corruption
Cc:

Description

By default the "Reclassify" box shown in Single Message View is empty (i.e. no bucket name is shown).

If the "Reclassify" button is clicked without selecting a bucket name then the "Single Message View" is replaced by the HISTORY page but this page no longer lists the message! This means the message cannot be viewed or reclassified.

The problem was introduced in POPFile 1.1.2 by changeset:3674 which changed /skins/default/view-page.thtml as follows:

r3453:      <option selected="selected"></option> 
r3674:      <option selected="selected">&nbsp;</option> 

This means that the message was reclassified to a bucket called ' ' and the corpus ends up with an empty bucketid field for this message. This means the UI cannot be used to undo the reclassification.

The following Perl errors were generated when the "Reclassify" button was clicked without selecting a bucket name:

Use of uninitialized value in addition (+) at c:\temp\112\basic/Classifier/Bayes.pm line 547.

Use of uninitialized value in addition (+) at c:\temp\112\basic/Classifier/Bayes.pm line 561.

Use of uninitialized value in sprintf at c:\temp\112\basic/UI/HTML.pm line 2192.

It is possible that something else was broken by that changeset, e.g. at least one similar change was made to another file.

Change History (6)

comment:1 by Brian Smith, 12 years ago

When I used grep to look for similar code I soon realised my initial analysis was incorrect!

Although removing the "&nbsp;" appeared to fix the problem I think the real problem is that the template code does not set an initial selection value.

If line 200 in /skins/default/view-page.thtml is changed as follows then using Single Message View's "Reclassify" button with no bucket selected simply returns the user to the HISTORY page without corrupting the corpus (i.e. all of the messages are still shown)

current code:   <option selected="selected">&nbsp;</option> 
revised code:   <option value="" selected="selected">&nbsp;</option>  

grep found another similar case in /skins/ocean/history-page.thtml:

    <select name="reclassify_<TMPL_VAR NAME="History_I">">
        <option selected="selected">&nbsp;</option>

        <TMPL_LOOP NAME="History_Loop_Loop_Buckets">

comment:2 by Brian Smith, 12 years ago

Owner: changed from Manni Heumann to Brian Smith
Status: newassigned

grep found another similar case in /skins/ocean/history-page.thtml

This is an even more serious bug - clicking "Reclassify" without selecting any bucket will clear out the 'bucketid' field for every message that was listed on the HISTORY page. This means those messages will disappear forever from the UI (and lots of Perl warnings will be generated).

grep has found another case that might need to be fixed: /skins/default/imap-bucket-folders.thtml line 16:

<option selected="selected"></option>

May need to change this line to

<option value="" selected="selected">&nbsp;</option>  

It seems that adding "&nbsp;" to remove HTML warnings can cause a problem if there is no default value specified for the option.

comment:3 by Brian Smith, 12 years ago

Summary: Clicking Single Message View's "Reclassify" button can corrupt the corpusClicking the "Reclassify" button can corrupt the database

Changed the title of this ticket to make it more accurate!

comment:4 by Brian Smith, 12 years ago

changeset:3817 fixes the Single Message View case (in the 'default' skin) and the problem with the HISTORY page in the 'ocean' skin. It also updated the 'ocean' skin's Single Message View to reflect the change to the 'default' skin.

comment:5 by Joseph Connors, 10 years ago

Resolution: fixed
Status: assignedclosed

comment:6 by Joseph Connors, 10 years ago

Resolution: fixed
Status: closedreopened
Version: 1.1.3svn

Reopened since we need to make this change on trunk also.

Note: See TracTickets for help on using tickets.