Merge lp:~googol-deactivatedaccount/openlp/bug-789143 into lp:openlp

Proposed by Andreas Preikschat
Status: Merged
Merged at revision: 1592
Proposed branch: lp:~googol-deactivatedaccount/openlp/bug-789143
Merge into: lp:openlp
Diff against target: 106 lines (+50/-45)
1 file modified
openlp/plugins/songs/lib/__init__.py (+50/-45)
To merge this branch: bzr merge lp:~googol-deactivatedaccount/openlp/bug-789143
Reviewer Review Type Date Requested Status
Jonathan Corwin (community) Approve
Raoul Snyman Approve
Tim Bentley Pending
Review via email: mp+62790@code.launchpad.net

This proposal supersedes a proposal from 2011-05-27.

Description of the change

Hello,

- "fixed" bug #789143

The "bug" lies in from_loose_input:

        verse_index = None
        if len(verse_name) > 1:
            verse_index = VerseType.from_translated_string(verse_name)
            if verse_index is None:
                verse_index = VerseType.from_string(verse_name)
        if verse_index is None:
            verse_index = VerseType.from_translated_tag(verse_name)
        if verse_index is None:
            verse_index = VerseType.from_tag(verse_name)
        return verse_index

If we pass "e" (Ending) to the method it first tries from_translated_tag (which is wrong). It seems that this causes problems when this method is used with English tags (but using a translated GUI).

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Jonathan Corwin (j-corwin) wrote : Posted in a previous version of this proposal

A bit worried about removing so much code, but your testing seems to have been thorough!

review: Approve
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) wrote :

I hope that this is a more secure way to do it. But no guarantee!

Revision history for this message
Raoul Snyman (raoul-snyman) wrote :

Looks better.

review: Approve
Revision history for this message
Jonathan Corwin (j-corwin) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/plugins/songs/lib/__init__.py'
2--- openlp/plugins/songs/lib/__init__.py 2011-05-26 17:11:22 +0000
3+++ openlp/plugins/songs/lib/__init__.py 2011-05-29 12:05:55 +0000
4@@ -265,52 +265,57 @@
5 whitespace = re.compile(r'\W+', re.UNICODE)
6 song.search_title = (whitespace.sub(u' ', song.title).strip() + u'@' +
7 whitespace.sub(u' ', song.alternate_title).strip()).strip().lower()
8- # Remove the old "language" attribute from lyrics tag (prior to 1.9.5). This
9- # is not very important, but this keeps the database clean. This can be
10- # removed when everybody has cleaned his songs.
11- song.lyrics = song.lyrics.replace(u'<lyrics language="en">', u'<lyrics>')
12- verses = SongXML().get_verses(song.lyrics)
13- lyrics = u' '.join([whitespace.sub(u' ', verse[1]) for verse in verses])
14- song.search_lyrics = lyrics.lower()
15- # We need a new and clean SongXML instance.
16- sxml = SongXML()
17- # Rebuild the song's verses, to remove any wrong verse names (for example
18- # translated ones), which might have been added prior to 1.9.5.
19- # List for later comparison.
20- compare_order = []
21- for verse in verses:
22- verse_type = VerseType.Tags[VerseType.from_loose_input(
23- verse[0][u'type'])]
24- sxml.add_verse_to_lyrics(
25- verse_type,
26- verse[0][u'label'],
27- verse[1],
28- verse[0][u'lang'] if verse[0].has_key(u'lang') else None
29- )
30- compare_order.append((u'%s%s' % (verse_type, verse[0][u'label'])
31- ).upper())
32- if verse[0][u'label'] == u'1':
33- compare_order.append(verse_type.upper())
34- song.lyrics = unicode(sxml.extract_xml(), u'utf-8')
35- # Rebuild the verse order, to convert translated verse tags, which might
36- # have been added prior to 1.9.5.
37- if song.verse_order:
38- order = song.verse_order.strip().split()
39- else:
40- order = []
41- new_order = []
42- for verse_def in order:
43- verse_type = VerseType.Tags[VerseType.from_loose_input(verse_def[0])]
44- if len(verse_def) > 1:
45- new_order.append((u'%s%s' % (verse_type, verse_def[1:])).upper())
46+ # Only do this, if we the song is a 1.9.4 song (or older).
47+ if song.lyrics.find(u'<lyrics language="en">') != -1:
48+ # Remove the old "language" attribute from lyrics tag (prior to 1.9.5).
49+ # This is not very important, but this keeps the database clean. This
50+ # can be removed when everybody has cleaned his songs.
51+ song.lyrics = song.lyrics.replace(
52+ u'<lyrics language="en">', u'<lyrics>')
53+ verses = SongXML().get_verses(song.lyrics)
54+ lyrics = u' '.join([whitespace.sub(u' ', verse[1]) for verse in verses])
55+ song.search_lyrics = lyrics.lower()
56+ # We need a new and clean SongXML instance.
57+ sxml = SongXML()
58+ # Rebuild the song's verses, to remove any wrong verse names (for
59+ # example translated ones), which might have been added prior to 1.9.5.
60+ # List for later comparison.
61+ compare_order = []
62+ for verse in verses:
63+ verse_type = VerseType.Tags[VerseType.from_loose_input(
64+ verse[0][u'type'])]
65+ sxml.add_verse_to_lyrics(
66+ verse_type,
67+ verse[0][u'label'],
68+ verse[1],
69+ verse[0][u'lang'] if verse[0].has_key(u'lang') else None
70+ )
71+ compare_order.append((u'%s%s' % (verse_type, verse[0][u'label'])
72+ ).upper())
73+ if verse[0][u'label'] == u'1':
74+ compare_order.append(verse_type.upper())
75+ song.lyrics = unicode(sxml.extract_xml(), u'utf-8')
76+ # Rebuild the verse order, to convert translated verse tags, which might
77+ # have been added prior to 1.9.5.
78+ if song.verse_order:
79+ order = song.verse_order.strip().split()
80 else:
81- new_order.append(verse_type.upper())
82- song.verse_order = u' '.join(new_order)
83- # Check if the verse order contains tags for verses which do not exist.
84- for order in new_order:
85- if order not in compare_order:
86- song.verse_order = u''
87- break
88+ order = []
89+ new_order = []
90+ for verse_def in order:
91+ verse_type = VerseType.Tags[
92+ VerseType.from_loose_input(verse_def[0])]
93+ if len(verse_def) > 1:
94+ new_order.append(
95+ (u'%s%s' % (verse_type, verse_def[1:])).upper())
96+ else:
97+ new_order.append(verse_type.upper())
98+ song.verse_order = u' '.join(new_order)
99+ # Check if the verse order contains tags for verses which do not exist.
100+ for order in new_order:
101+ if order not in compare_order:
102+ song.verse_order = u''
103+ break
104 # The song does not have any author, add one.
105 if not song.authors:
106 name = SongStrings.AuthorUnknown