Bug 6458 - incorrect parsing result in translation processing
Summary: incorrect parsing result in translation processing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: 3.4
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Frédéric Demians
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 8407 18692
  Show dependency treegraph
 
Reported: 2011-06-04 14:12 UTC by Sébastien Marie
Modified: 2017-05-29 14:29 UTC (History)
2 users (show)

See Also:
Change sponsored?: Seeking developer
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (2.90 KB, patch)
2011-07-21 08:28 UTC, Frédéric Demians
Details | Diff | Splinter Review
Amended proposed patch (2.90 KB, patch)
2011-07-21 11:06 UTC, Frédéric Demians
Details | Diff | Splinter Review
Proposed patch (2) (32.88 KB, patch)
2011-07-21 11:44 UTC, Frédéric Demians
Details | Diff | Splinter Review
Proposed patch for testing and signing off (37.06 KB, patch)
2011-07-23 14:40 UTC, Frédéric Demians
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6458 Template Toolkit files test case (21.05 KB, patch)
2011-08-11 16:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect parsing result in translation processing (2.97 KB, patch)
2011-08-20 16:34 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / sco-main.tt (2.27 KB, patch)
2011-08-20 17:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing - masthead.inc (1.47 KB, patch)
2011-08-20 17:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing - doc-head-close/open.inc (2.43 KB, patch)
2011-08-21 22:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect parsing result in translation processing (3.08 KB, patch)
2011-08-22 03:46 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / sco-main.tt (2.36 KB, patch)
2011-08-22 03:48 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing - masthead.inc (1.55 KB, patch)
2011-08-22 03:49 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing - doc-head-close/open.inc (3.44 KB, patch)
2011-08-22 06:25 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / opac-detail.tt (3.04 KB, patch)
2011-08-22 06:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / overdue.tt (2.60 KB, patch)
2011-08-22 20:49 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / review.tt (3.06 KB, patch)
2011-08-22 21:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / authorities.tt (1.92 KB, patch)
2011-08-22 21:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / 3x reports/... (3.67 KB, patch)
2011-08-22 21:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / acqui/... (6.36 KB, patch)
2011-08-22 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / cataloguing/... (5.12 KB, patch)
2011-08-22 22:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / cataloguing/... (5.27 KB, patch)
2011-08-22 22:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing - rotating_collections/... (2.87 KB, patch)
2011-08-24 17:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Signed off patch correcting doc-head-close.inc and doc-head-open.inc (3.50 KB, patch)
2011-08-24 20:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting overdue.tt (2.66 KB, patch)
2011-08-24 20:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed-off patch correcting review.tt (3.12 KB, patch)
2011-08-24 20:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed-off patch correcting authorities.tt (1.98 KB, patch)
2011-08-24 20:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting 3x reports (3.73 KB, patch)
2011-08-24 20:11 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting masthead.inc (1.66 KB, patch)
2011-08-24 20:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting acqui templates (6.42 KB, patch)
2011-08-24 20:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting cataloging templates (5.33 KB, patch)
2011-08-24 20:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / serials/... (3.19 KB, patch)
2011-08-24 20:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Signed off patch correcting opac-detail.tt (3.10 KB, patch)
2011-08-25 16:54 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting rotating collections (2.93 KB, patch)
2011-08-25 17:13 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting serials templates (3.25 KB, patch)
2011-08-25 17:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed-off patch correcting sco-main.tt (2.46 KB, patch)
2011-08-25 17:49 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / suggestion (6.72 KB, patch)
2011-08-25 22:53 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 6458: Incorrect translation processing / results (4.79 KB, patch)
2011-08-25 22:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Proposed fix for opac-reserve.tt (3.65 KB, patch)
2011-08-26 13:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting suggestions (6.79 KB, patch)
2011-08-26 13:45 UTC, Owen Leonard
Details | Diff | Splinter Review
Signed off patch correcting results.tt (4.85 KB, patch)
2011-08-26 13:54 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for opac-messaing.tt (5.25 KB, patch)
2011-08-26 14:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for memberentrygen.tt (28.32 KB, patch)
2011-08-26 14:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for patron-attr-types.tt (7.37 KB, patch)
2011-08-26 14:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for admin/clone-rules.tt (2.05 KB, patch)
2011-08-26 19:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for admin/branches.tt (2.65 KB, patch)
2011-08-26 20:11 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for admin/branch_transfer_limits.tt (1.86 KB, patch)
2011-08-26 20:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for header.inc (1.32 KB, patch)
2011-09-01 13:47 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for admin/aqplan.tt (1.77 KB, patch)
2011-09-01 14:14 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for moremember.tt (1.83 KB, patch)
2011-09-01 14:28 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for labels/spinelabel-print.tt (1.43 KB, patch)
2011-09-01 14:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for admin/systempreferences.tt (10.26 KB, patch)
2011-09-01 14:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for addbiblio.tt (3.71 KB, patch)
2011-09-01 15:17 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / addbiblio (4.02 KB, patch)
2011-09-02 06:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / systempreferences (10.33 KB, patch)
2011-09-02 06:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / memberentrygen (28.47 KB, patch)
2011-09-02 12:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Fix for Bug 6458 Follow-up for memberentrygen.tt (1.38 KB, patch)
2011-09-02 12:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Fix for Bug 6458 - incorrect parsing result in translation processing (7.49 KB, patch)
2011-09-02 12:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / patron-attr-types (7.52 KB, patch)
2011-09-02 12:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / opac-reserve (3.71 KB, patch)
2011-09-02 14:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Proposed fix for tools/manage-marc-import.tt (1.55 KB, patch)
2011-09-02 14:21 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / clone-rules (2.13 KB, patch)
2011-09-02 18:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6458 - incorrect parsing result in translation processing / branches (2.73 KB, patch)
2011-09-02 19:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Proposed fix for budgets-admin-search.inc (1.46 KB, patch)
2011-09-13 14:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for tools/csv-profiles.tt (2.49 KB, patch)
2011-09-13 14:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Follow-up fix for addbiblio.tt (1.93 KB, patch)
2011-09-13 16:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Follow-up fix for Bug 6458, incorrect parsing result in translation processing (1.94 KB, patch)
2011-09-13 22:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Follow-up fix for Bug 6458, incorrect parsing result in translation processing (deleted)
2011-09-13 22:13 UTC, Chris Cormack
Details
Proposed fix for doc-head-open.inc (1.21 KB, patch)
2011-09-14 13:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Revised fix for addbiblio.tt (2.51 KB, patch)
2011-09-14 13:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for messaging preference form (6.75 KB, patch)
2011-09-14 19:23 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sébastien Marie 2011-06-04 14:12:54 UTC
The translation processing issue no error (translate install fr-FR), but the resulting page for almost one template is incorrect (I don't speak about translated string, but structure of resulting file).

In 'koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt', line 135:

<input type="button" value="Renew Item" [% UNLESS ( renew ) %] name="confirm"[% END %] class="renew" onclick="this.form.confirmed.value='1';this.form.submit();" />

The result, 'koha-tmpl/opac-tmpl/prog/fr-FR/modules/sco/sco-main.tt', line 88:

<input value="Renouveler document" name="confirm" unless="UNLESS" %]="%]" renew="renew" end="END" )=")" type="button" onclick="this.form.confirmed.value='1';this.form.submit();" class="renew" (="(" [%="[%" />

This kind of error arrived two times in 'sco/sco-main.tt'.

Note: The result is in reality more complex: 7 lines of 'en' template are mixed in 1 line in 'fr-FR' result template.
Comment 1 Sébastien Marie 2011-06-05 12:12:24 UTC
It seems the problem is in 'misc/translator/TTParser.pm', in the processing of tag (function 'start', for handling opening html tags).

It isn't expected to have "template text" at the tag level:
<tagname attr1="value1" templateText attr2="value2" ...>

So TTParser interprete this as:
<tagname attr1="value1" templateText="templateText" attr2="value2" ...>

In the reported example:
<input type="button" value="Renew Item" [% UNLESS ( renew ) %] name="confirm"[%
END %] class="renew"
onclick="this.form.confirmed.value='1';this.form.submit();" />

TTParser read as:
 tagname=input
 attributes = {



In template, the two only occurrences I found in this scheme are in 'modules/sco/sco-main.tt'...
Comment 2 Sébastien Marie 2011-06-05 12:15:50 UTC
sorry for comments before: tabulation don't work well in textarea, and the next focused item is 'Save Changes' button...

Redo :

<input type="button" value="Renew Item" [% UNLESS ( renew ) %] name="confirm"[%
END %] class="renew"
onclick="this.form.confirmed.value='1';this.form.submit();" />

TTParser read as:
 tagname=input
 attributes = {
   type = "button",
   value = "Renew Item",
   [% = "[%",
   unless = "UNLESS",
   %] = "%]",
   ...
}

The re-ordoring at the end is due to order in hashtable...
Comment 3 Sébastien Marie 2011-06-05 13:53:00 UTC
I have found few others cases with Template Directive in tag, but where there are not problem with transformation.

- in 'opac-user.tt'
<tr[% IF ( overdue ) %] class="overdue"[% END %]>

- in 'opac-detail.tt'
<li[% IF ( defaulttab == 'subscriptions' ) %] class="ui-tabs-selected"[% END %]>

The difference seems to be that here, there are not string to be translated.

So problem may be corrected in 'tmpl_process3.pl': function 'text_replace_tag'.

Currently, if translation is found, the HTML element is "rewrited" based on attributes parsed (where directives are not tagged as DIRECTIVE, and in HASHTABLE [so order is lost]).

So two solutions:
 - work in 'TTParser.pm' for correctly tag DIRECTIVE in tag-element, and processing accordingly in 'text_replace_tag'

 - work in 'tmpl_process3.pl' for another method (like regex, but it not a very strong method...)
Comment 4 Sébastien Marie 2011-07-19 09:50:52 UTC
here, a discussion about his problem:
http://lists.koha-community.org/pipermail/koha-devel/2011-July/035917.html
Comment 5 Frédéric Demians 2011-07-21 06:48:12 UTC
(In reply to comment #4)
> here, a discussion about his problem:
> http://lists.koha-community.org/pipermail/koha-devel/2011-July/035917.html

Could list html tags within which TT directives can be found? I would like to port your test case, bug I'm not very fluent in haskell...
Comment 6 Sébastien Marie 2011-07-21 07:12:56 UTC
Currently, the parser search the string "[%" in *every* HTML tag.

But it isn't a HTML parser. It just track when the string "[%" in placed after the string "<" (and not ">" between "<" and "[%"), and not between two string '"'. (but need to track also CDATA and COMMENT because of false-positive else).

It follow a *very simplified* HTML syntax for parsing.

It is relatively simple (and commented), and based on a "context" concept: it just defined rules for enter in a "context", and to change the "context", when a specific string is encounter.

For the INITIAL context (the default one):
 - when "<![CDATA[" string is encounter: go in CDATA context
 - when "<!--" string is encounter: go in COMMENT context
 - when "<" string encounter: go in TAG context

For the TAG context:
 - when encounter the string '"' : enter in STRING context
 - when encounter the string "[%" : mark a problem, as a template directive is found in tag, but not in an attribute (because an attribute is a STRING context)
 - when encounter the string ">": go in INITIAL context

For the STRING context:
 - when encounter the string '\"' : keep the STRING context
 - when encounter only the string '"' : go in TAG context

(in CDATA context, how to go in INITIAL context : "]]>" encounter ;
 or in COMMENT context: "-->"
)

That's all... (but the parser keep track the line number for reporting).

I hope I answer your question, and that I have clarified how work the parser...
Comment 7 Frédéric Demians 2011-07-21 08:28:36 UTC Comment hidden (obsolete)
Comment 8 Sébastien Marie 2011-07-21 09:36:24 UTC
(In reply to comment #7)
> Created attachment 4695 [details] [review]
> Proposed patch
> 
> Could you try this test case and check that it detects the appropriate TT
> issues?

Issues are incomplete: haskell version report 269 issues in 45 files, versus perl 34 issues in 14 files.

The list could be consulted here: http://lists.koha-community.org/pipermail/koha-devel/2011-July/035922.html , in the attachment (but *.inc are not included, and "269 in 45 files" is the last count on master repo)

There are multiples problems with this perl version:
 - it is line oriented versus stream oriented. So in case of opening tag on one line, and inclusion of directive in other line, the issue is not reported.
(for examples:
  see ./koha-tmpl/opac-tmpl/prog/en/modules/opac-messaging.tt, line 74
  see ./koha-tmpl/opac-tmpl/prog/en/includes/doc-head-close.inc, line 42
)

 - it only check directive near the tagname (like <li[%IF ), and not more general construct (like <option value="10"[%IF )
(for examples:
  see ./koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt, line 135
  see ./koha-tmpl/intranet-tmpl/prog/en/modules/admin/systempreferences.tt, line 91
)
Comment 9 Frédéric Demians 2011-07-21 10:38:04 UTC
> There are multiples problems with this perl version:

You're correct. I have no solution in Perl, neither time. We have
TTParser module. So we should use it to parse properly templates and
identify invalid constructions.
Comment 10 Frédéric Demians 2011-07-21 11:06:25 UTC Comment hidden (obsolete)
Comment 11 Frédéric Demians 2011-07-21 11:44:28 UTC Comment hidden (obsolete)
Comment 12 Sébastien Marie 2011-07-23 08:28:56 UTC
(In reply to comment #11)
> Created attachment 4699 [details] [review]
> Proposed patch (2)

This version works very well. There are some differences between the haskell version and the perl one, but there are not generally problematic:
 - problems not related of TT template
    - haskell version report also problems with CDATA section, partial quoting attribute (and a false-positive for use of single-quoting in attributes)

 - differences but not a problem
    - haskell version report multiple errors for a tag (if multiple directive inclusion)
    - haskell version report line number of the directive, although perl version report line number of the beginning of tag with directive.

The only amelioration to do, is to include the *.inc for parsing: some of them included directive in tag... else perfect !

So in don't sign-off, waiting the corrected version; and I also need more time to check for side-effect due to move of TTParser files in C4.

Thanks.
Comment 13 Frédéric Demians 2011-07-23 14:40:32 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2011-08-11 16:07:36 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2011-08-18 09:13:46 UTC
Pushed.
Comment 16 Katrin Fischer 2011-08-20 16:34:11 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2011-08-20 17:05:17 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2011-08-20 17:44:12 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2011-08-21 22:14:01 UTC Comment hidden (obsolete)
Comment 20 Chris Cormack 2011-08-22 03:46:21 UTC Comment hidden (obsolete)
Comment 21 Chris Cormack 2011-08-22 03:48:02 UTC Comment hidden (obsolete)
Comment 22 Chris Cormack 2011-08-22 03:49:10 UTC Comment hidden (obsolete)
Comment 23 Chris Cormack 2011-08-22 03:52:00 UTC
I have signed off on 3 of these patches, but the doc-head-close and open still needs a bit more work

Before

# opac-tmpl/prog/en/includes/doc-head-close.inc: 41, 77
# opac-tmpl/prog/en/includes/doc-head-open.inc: 3

After
# opac-tmpl/prog/en/includes/doc-head-close.inc: 80

So still needs some work
Comment 24 Sébastien Marie 2011-08-22 04:34:58 UTC
Comment on attachment 5078 [details] [review]
Bug 6458: Incorrect parsing result in translation processing

Hi,

The patch on 'opac-detail.tt' don't preserve the semantic, of original IF/ELSE clause: in the correction the IF clause and the ELSE clause are the same...

The code is currently:
+[% IF ( defaulttab == 'holdings' ) %]<li class="ui-tabs-selected">
+[% ELSE %]<li class="ui-tabs-selected">[% END %]

But it should be:
+[% IF ( defaulttab == 'holdings' ) %]<li class="ui-tabs-selected">
+[% ELSE %]<li>[% END %]

The same problem occurs for all corrections in opac-detail.tt (but others patchs seems good).
Comment 25 Katrin Fischer 2011-08-22 06:25:32 UTC Comment hidden (obsolete)
Comment 26 Katrin Fischer 2011-08-22 06:30:42 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2011-08-22 20:49:30 UTC Comment hidden (obsolete)
Comment 28 Katrin Fischer 2011-08-22 21:01:22 UTC Comment hidden (obsolete)
Comment 29 Katrin Fischer 2011-08-22 21:17:15 UTC Comment hidden (obsolete)
Comment 30 Katrin Fischer 2011-08-22 21:29:12 UTC Comment hidden (obsolete)
Comment 31 Katrin Fischer 2011-08-22 21:52:10 UTC Comment hidden (obsolete)
Comment 32 Katrin Fischer 2011-08-22 22:14:14 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2011-08-22 22:22:10 UTC Comment hidden (obsolete)
Comment 34 Katrin Fischer 2011-08-24 17:51:42 UTC Comment hidden (obsolete)
Comment 35 Owen Leonard 2011-08-24 20:06:47 UTC Comment hidden (obsolete)
Comment 36 Owen Leonard 2011-08-24 20:08:02 UTC Comment hidden (obsolete)
Comment 37 Owen Leonard 2011-08-24 20:09:11 UTC Comment hidden (obsolete)
Comment 38 Owen Leonard 2011-08-24 20:10:02 UTC Comment hidden (obsolete)
Comment 39 Owen Leonard 2011-08-24 20:11:03 UTC Comment hidden (obsolete)
Comment 40 Owen Leonard 2011-08-24 20:15:46 UTC Comment hidden (obsolete)
Comment 41 Owen Leonard 2011-08-24 20:20:55 UTC Comment hidden (obsolete)
Comment 42 Owen Leonard 2011-08-24 20:26:15 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2011-08-24 20:39:41 UTC Comment hidden (obsolete)
Comment 44 Owen Leonard 2011-08-25 16:54:47 UTC Comment hidden (obsolete)
Comment 45 Owen Leonard 2011-08-25 17:13:29 UTC Comment hidden (obsolete)
Comment 46 Owen Leonard 2011-08-25 17:22:49 UTC Comment hidden (obsolete)
Comment 47 Owen Leonard 2011-08-25 17:49:32 UTC Comment hidden (obsolete)
Comment 48 Owen Leonard 2011-08-25 19:50:18 UTC
Working on memberentrygen.tt. Somebody please stop me if they are too!
Comment 49 Katrin Fischer 2011-08-25 22:53:43 UTC Comment hidden (obsolete)
Comment 50 Katrin Fischer 2011-08-25 22:54:33 UTC Comment hidden (obsolete)
Comment 51 Owen Leonard 2011-08-26 13:33:35 UTC Comment hidden (obsolete)
Comment 52 Owen Leonard 2011-08-26 13:45:28 UTC Comment hidden (obsolete)
Comment 53 Owen Leonard 2011-08-26 13:54:53 UTC Comment hidden (obsolete)
Comment 54 Owen Leonard 2011-08-26 14:06:39 UTC
Created attachment 5158 [details] [review]
Proposed fix for opac-messaing.tt

Correction for opac-messaging.tt tops tag attribute markup from being passed to the template directly in favor of using template logic.
Comment 55 Owen Leonard 2011-08-26 14:23:11 UTC Comment hidden (obsolete)
Comment 56 Owen Leonard 2011-08-26 14:59:08 UTC Comment hidden (obsolete)
Comment 57 Owen Leonard 2011-08-26 19:59:19 UTC Comment hidden (obsolete)
Comment 58 Owen Leonard 2011-08-26 20:11:11 UTC Comment hidden (obsolete)
Comment 59 Owen Leonard 2011-08-26 20:18:28 UTC Comment hidden (obsolete)
Comment 60 Sébastien Marie 2011-08-27 07:18:57 UTC
Comment on attachment 5156 [details] [review]
Signed off patch correcting suggestions

In the patch 'suggestion/suggestion.tt', in the second block of corrections, the semantic seems to not be preserved: 

Here, just an example, for the first line (of second block):
Orign: [% IF ( suggestion.statusselected_ASKED ) %]
Patch: [% IF (statusselected_ASKED ) %]
Comment 61 Owen Leonard 2011-09-01 13:47:04 UTC Comment hidden (obsolete)
Comment 62 Owen Leonard 2011-09-01 14:14:03 UTC Comment hidden (obsolete)
Comment 63 Owen Leonard 2011-09-01 14:28:56 UTC Comment hidden (obsolete)
Comment 64 Owen Leonard 2011-09-01 14:39:10 UTC Comment hidden (obsolete)
Comment 65 Owen Leonard 2011-09-01 14:56:36 UTC Comment hidden (obsolete)
Comment 66 Owen Leonard 2011-09-01 15:02:18 UTC
Comment on attachment 5158 [details] [review]
Proposed fix for opac-messaing.tt

This patch should be combined with one for includes/messaging-preference-form.inc. I'll submit a new patch.
Comment 67 Owen Leonard 2011-09-01 15:17:50 UTC Comment hidden (obsolete)
Comment 68 Owen Leonard 2011-09-01 15:21:05 UTC
Whoops, forgot that some fixes had already been applied to addbiblio.tt. That last one may not apply in conjunction with attachment 5132 [details] [review].
Comment 69 Katrin Fischer 2011-09-02 06:07:21 UTC
STATUS UPDATE

Assuming all patches proposed patches get signed off, this is the current output of xt/tt_valid.t:

# Files list: 
# opac-tmpl/prog/en/modules/opac-messaging.tt: 62, 71, 81
# intranet-tmpl/prog/en/modules/tools/csv-profiles.tt: 104
# intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt: 160
# intranet-tmpl/prog/en/includes/messaging-preference-form.inc: 57, 62, 75, 80, 93, 98
# intranet-tmpl/prog/en/includes/doc-head-open.inc: 3
# intranet-tmpl/prog/en/includes/budgets-admin-search.inc: 64
Comment 70 Katrin Fischer 2011-09-02 06:12:42 UTC Comment hidden (obsolete)
Comment 71 Katrin Fischer 2011-09-02 06:21:47 UTC Comment hidden (obsolete)
Comment 72 Katrin Fischer 2011-09-02 12:16:57 UTC Comment hidden (obsolete)
Comment 73 Katrin Fischer 2011-09-02 12:17:16 UTC Comment hidden (obsolete)
Comment 74 Katrin Fischer 2011-09-02 12:35:25 UTC Comment hidden (obsolete)
Comment 75 Katrin Fischer 2011-09-02 12:37:41 UTC Comment hidden (obsolete)
Comment 76 Owen Leonard 2011-09-02 14:03:20 UTC
(In reply to comment #60)
> In the patch 'suggestion/suggestion.tt', in the second block of corrections,
> the semantic seems to not be preserved: 
> 
> Here, just an example, for the first line (of second block):
> Orign: [% IF ( suggestion.statusselected_ASKED ) %]
> Patch: [% IF (statusselected_ASKED ) %]

I think this modification is correct. Line 305 of suggestion.pl: 

$template->param( "statusselected_$$suggestion_ref{'STATUS'}"  =>1);

It doesn't look like that variable needs any scope. My search of the source indicates that there isn't any other place where a variable with that name is created.

The behavior of the page seems to be the same with our without the variable scope, and I think this is because the variable never evaluates as true. I think the logic around that <option> is unused. I can't see what circumstances would lead to that status having a preselected value.
Comment 77 Katrin Fischer 2011-09-02 14:04:37 UTC Comment hidden (obsolete)
Comment 78 Owen Leonard 2011-09-02 14:21:11 UTC Comment hidden (obsolete)
Comment 79 Katrin Fischer 2011-09-02 18:52:25 UTC Comment hidden (obsolete)
Comment 80 Katrin Fischer 2011-09-02 19:18:23 UTC Comment hidden (obsolete)
Comment 81 Chris Cormack 2011-09-12 21:35:36 UTC
I have pushed up to Signed off patch correcting results.tt, please sign off the others and I will push the rest
Comment 82 Owen Leonard 2011-09-13 14:06:24 UTC Comment hidden (obsolete)
Comment 83 Owen Leonard 2011-09-13 14:09:17 UTC Comment hidden (obsolete)
Comment 84 Owen Leonard 2011-09-13 16:33:02 UTC Comment hidden (obsolete)
Comment 85 Chris Cormack 2011-09-13 22:12:40 UTC Comment hidden (obsolete)
Comment 86 Chris Cormack 2011-09-13 22:13:00 UTC Comment hidden (obsolete)
Comment 87 Chris Cormack 2011-09-13 22:13:25 UTC
The content of attachment 5420 [details] has been deleted by
    Chris Cormack <chris@bigballofwax.co.nz>
without providing any reason.

The token used to delete this attachment was generated at 2011-09-14 10:13:19 NZST.
Comment 88 Chris Cormack 2011-09-13 22:14:00 UTC
I have pushed all the patches attached to this, apart from the one left which does not apply.
Comment 89 Chris Cormack 2011-09-13 22:15:37 UTC
not ok 1 - TT syntax: not using TT directive within HTML tag
#   Failed test 'TT syntax: not using TT directive within HTML tag'
#   at xt/tt_valid.t line 55.
# Files list: 
# opac-tmpl/prog/en/modules/opac-messaging.tt: 62, 71, 81
# intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: 782
# intranet-tmpl/prog/en/includes/messaging-preference-form.inc: 57, 62, 75, 80, 93, 98
# intranet-tmpl/prog/en/includes/doc-head-open.inc: 3


Here are the things left failing
Comment 90 Owen Leonard 2011-09-13 23:08:08 UTC
I have patches ready to submit for the last few. Will do so ASAP.
Comment 91 Owen Leonard 2011-09-14 12:50:27 UTC
Comment on attachment 5158 [details] [review]
Proposed fix for opac-messaing.tt

I'm removing obsolete status from this patch because it doesn't look like it got pushed.
Comment 92 Owen Leonard 2011-09-14 13:19:42 UTC
Created attachment 5425 [details] [review]
Proposed fix for doc-head-open.inc

Correcting doc-head-open.inc (again?)
Comment 93 Owen Leonard 2011-09-14 13:23:57 UTC
Created attachment 5426 [details] [review]
Revised fix for addbiblio.tt
Comment 94 Owen Leonard 2011-09-14 19:23:33 UTC
Created attachment 5427 [details] [review]
Proposed fix for messaging preference form

Fixing improperly nested template logic inside HTML tags in messaging preferences include.

NOTE: This patch requires the patch for opac-messaging.tt, which includes a change to C4/Form/MessagingPreferences.pm
Comment 95 Chris Cormack 2011-09-14 20:42:10 UTC
Pushed, all TT tests now passing WOOT!
Comment 96 Chris Nighswonger 2011-10-26 18:02:12 UTC
The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.
Comment 97 Sébastien Marie 2011-10-27 04:19:30 UTC
(In reply to comment #96)
> The fix for this bug was published in the 3.4.5 release. If you were the
> reporter of this bug, please take time to verify the fix and update the status
> of this bug report accordingly. If the bug is fixed to your satisfaction,
> please close this report.

Thanks.