aboutsummaryrefslogtreecommitdiff
path: root/doc/plugins/po/discussion.mdwn
blob: 6a7bb7f4bbf073aa4ca134327be139f8b8147d3b (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
[[!toc ]]

----

# Security review

## Probable holes

_(The list of things to fix.)_

### po4a-gettextize

* po4a CVS 2009-01-16
* Perl 5.10.0

`po4a-gettextize` uses more or less the same po4a features as our
`refreshpot` function.

Without specifying an input charset, zzuf'ed `po4a-gettextize` quickly
errors out, complaining it was not able to detect the input charset;
it leaves no incomplete file on disk. I therefore had to pretend the
input was in UTF-8, as does the po plugin.

        zzuf -c -s 13 -r 0.1 \
            po4a-gettextize -f text -o markdown -M utf-8 -L utf-8 \
             -m GPL-3 -p GPL-3.pot

Crashes with:

        Malformed UTF-8 character (UTF-16 surrogate 0xdfa4) in substitution
        iterator at /usr/share/perl5/Locale/Po4a/Po.pm line 1449.
        Malformed UTF-8 character (fatal) at /usr/share/perl5/Locale/Po4a/Po.pm
        line 1449.

An incomplete pot file is left on disk. Unfortunately Po.pm tells us
nothing about the place where the crash happens.

> It's fairly standard perl behavior when fed malformed utf-8. As long
> as it doesn't crash ikiwiki, it's probably acceptable. Ikiwiki can
> do some similar things itself when fed malformed utf-8 (doesn't
> crash tho) --[[Joey]]

----

## Potential gotchas

_(Things not to do.)_


### Blindly activating more po4a format modules

The format modules we want to use have to be checked, as not all are
safe (e.g. the LaTeX module's behaviour is changed by commands
included in the content); they may use regexps generated from
the content.

----

## Hopefully non-holes

_(AKA, the assumptions that will be the root of most security holes...)_

### PO file features

No [documented](http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files)
directive that can be put in po files is supposed to cause mischief
(ie, include other files, run commands, crash gettext, whatever).

### gettext

#### Security history

The only past security issue I could find in GNU gettext is
[CVE-2004-0966](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2004-0966),
*i.e.* [Debian bug #278283](http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=278283):
the autopoint and gettextize scripts in the GNU gettext package (1.14
and later versions) may allow local users to overwrite files via
a symlink attack on temporary files.

This plugin would not have allowed to exploit this bug, as it does not
use, either directly or indirectly, the faulty scripts.

Note: the lack of found security issues can either indicate that there
are none, or reveal that no-one ever bothered to find or publish them.

#### msgmerge

`refreshpofiles()` runs this external program.

* I was not able to crash it with `zzuf`.
* I could not find any past security hole.

#### msgfmt

`isvalidpo()` runs this external program.

* I was not able to make it behave badly using zzuf: it exits cleanly
  when too many errors are detected.
* I could not find any past security hole.

### po4a

#### Security history

The only past security issue I could find in po4a is
[CVE-2007-4462](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-4462):
`lib/Locale/Po4a/Po.pm` in po4a before 0.32 allowed local users to
overwrite arbitrary files via a symlink attack on the
gettextization.failed.po temporary file.

This plugin would not have allowed to exploit this bug, as it does not
use, either directly or indirectly, the faulty `gettextize` function.

Note: the lack of found security issues can either indicate that there
are none, or reveal that no-one ever bothered to find or publish them.

#### General feeling

Are there any security issues on running po4a on untrusted content?

To say the least, this issue is not well covered, at least publicly:

* the documentation does not talk about it;
* grep'ing the source code for `security` or `trust` gives no answer.

On the other hand, a po4a developer answered my questions in
a convincing manner, stating that processing untrusted content was not
an initial goal, and analysing in detail the possible issues.
The following analysis was done with his help.

#### Details

* the core (`Po.pm`, `Transtractor.pm`) should be safe
* po4a source code was fully checked for other potential symlink
  attacks, after discovery of one such issue
* the only external program run by the core is `diff`, in `Po.pm` (in
  parts of its code we don't use)
* `Locale::gettext` is only used to display translated error messages
* Nicolas François "hopes" `DynaLoader` is safe, and has "no reason to
  think that `Encode` is not safe"
* Nicolas François has "no reason to think that `Encode::Guess` is not
  safe". The po plugin nevertheless avoids using it by defining the
  input charset (`file_in_charset`) before asking `TransTractor` to
  read any file. NB: this hack depends on po4a internals.

##### Locale::Po4a::Text

* does not run any external program
* only `do_paragraph()` builds regexp's that expand untrusted
  variables; according to [[Joey]], this is "Freaky code, but seems ok
  due to use of `quotementa`".

##### Text::WrapI18N

`Text::WrapI18N` can cause DoS
([Debian bug #470250](http://bugs.debian.org/470250)).
It is optional, and we do not need the features it provides.

If a recent enough po4a (>=0.35) is installed, this module's use is
fully disabled. Else, the wiki administrator is warned about this
at runtime.

##### Term::ReadKey

`Term::ReadKey` is not a hard dependency in our case, *i.e.* po4a
works nicely without it. But the po4a Debian package recommends
`libterm-readkey-perl`, so it will probably be installed on most
systems using the po plugin.

`Term::ReadKey` has too far reaching implications for us to
be able to guarantee anything wrt. security.

If a recent enough po4a (>=2009-01-15 CVS, which will probably be
released as 0.35) is installed, this module's use is fully disabled.

##### Fuzzing input

###### po4a-translate

* po4a CVS 2009-01-16
* Perl 5.10.0

`po4a-translate` uses more or less the same po4a features as our
`filter` function.

Without specifying an input charset, same behaviour as
`po4a-gettextize`, so let's specify UTF-8 as input charset as of now.

`LICENSES` is a 21M file containing 100 concatenated copies of all the
files in `/usr/share/common-licenses/`; I had no existing PO file or
translated versions at hand, which renders these tests
quite incomplete.

        zzuf -cv -s 0:10 -r 0.001:0.3 \
          po4a-translate -d -f text -o markdown -M utf-8 -L utf-8 \
            -k 0 -m LICENSES -p LICENSES.fr.po -l test.fr

... seems to lose the fight, at the `readpo(LICENSES.fr.po)` step,
against some kind of infinite loop, deadlock, or any similar beast.

The root of this bug lies in `Text::WrapI18N`, see the corresponding
section.


----

## Fixed holes


----

# original contrib/po page, with old commentary

I've been working on a plugin called "po", that adds support for multi-lingual wikis,
translated with gettext, using [po4a](http://po4a.alioth.debian.org/).

More information:

* It can be found in my "po" branch:
  `git clone git://gaffer.ptitcanardnoir.org/ikiwiki.git`
* It is self-contained, *i.e.* it does not modify ikiwiki core at all.
* It is documented (including TODO and plans for next work steps) in
  `doc/plugins/po.mdwn`, which can be found in the same branch.
* No public demo site is available so far, I'm working on this.

My plan is to get this plugin clean enough to be included in ikiwiki.

The current version is a proof-of-concept, mature enough for me to dare submitting it here,
but I'm prepared to hear various helpful remarks, and to rewrite parts of it as needed.

Any thoughts on this?

> Well, I think it's pretty stunning what you've done here. Seems very
> complete and well thought out. I have not read the code in great detail
> yet.
> 
> Just using po files is an approach I've never seen tried with a wiki. I
> suspect it will work better for some wikis than others. For wikis that
> just want translations that match the master language as closely as
> possible and don't wander off and diverge, it seems perfect. (But what happens
> if someone edits the Discussion page of a translated page?)
> 
> Please keep me posted, when you get closer to having all issues solved
> and ready for merging I can do a review and hopefully help with the
> security items you listed. --[[Joey]]

>> Thanks a lot for your quick review, it's reassuring to hear such nice words
>> from you. I did not want to design and write a full translation system, when
>> tools such as gettext/po4a already have all the needed functionality, for cases
>> where the master/slave languages paradigm fits.
>> Integrating these tools into ikiwiki plugin system was a pleasure.
>>
>> I'll tell you when I'm ready for merging, but in the meantime,
>> I'd like you to review the changes I did to the core (3 added hooks).
>> Can you please do this? If not, I'll go on and hope I'm not going to far in
>> the wrong direction.
>>
>>> Sure.. I'm not completly happy with any of the hooks since they're very
>>> special purpose, and also since `run_hooks` is not the best interface
>>> for a hook that modifies a variable, where only the last hook run will
>>> actually do anything. It might be better to just wrap
>>> `targetpage`, `bestlink`, and `beautify_urlpath`. But, I noticed
>>> the other day that such wrappers around exported functions are only visible by
>>> plugins loaded after the plugin that defines them.
>>> 
>>> Update: Take a look at the new "Function overriding" section of
>>> [[plugins/write]]. I think you can just inject wrappers about a few ikiwiki
>>> functions, rather than adding hooks. The `inject` function is pretty
>>> insane^Wlow level, but seems to work great. --[[Joey]]
>>>
>>>> Thanks a lot, it seems to be a nice interface for what I was trying to achieve.
>>>> I may be forced to wait two long weeks before I have a chance to confirm
>>>> this. Stay tuned. --[[intrigeri]]
>>>>
>>>>> I've updated the plugin to use `inject`. It is now fully self-contained,
>>>>> and does not modify the core anymore. --[[intrigeri]]
>>
>> The Discussion pages issue is something I am not sure about yet. But I will
>> probably decide that "slave" pages, being only translations, don't deserve
>> a discussion page: the discussion should happen in the language in which the
>> pages are written for real, which is the "master" one. --[[intrigeri]]
>> 
>> I think that's a good decision, you don't want to translate discussion,
>> and if the discussion page turns out multilingual, well, se la vi. ;-)
>> 
>> Relatedly, what happens if a translated page has a broken link, and you
>> click on it to edit it? Seems you'd first have to create a master page
>> and could only then translate it, right? I wonder if this will be clear
>> though to the user.
>>
>>> Right: a broken link points to the URL that allows to create
>>> a page that can either be a new master page or a non-translatable
>>> page, depending on `po_translatable_pages` value. The best
>>> solution I can thing of is to use [[plugins/edittemplate]] to
>>> insert something like "Warning: this is a master page, that must
>>> be written in $MASTER_LANGUAGE" into newly created master pages,
>>> and maybe another warning message on newly created
>>> non-translatable pages. It seems quite doable to me, but in order
>>> to avoid breaking existing functionality, it implies to hack a bit
>>> [[plugins/edittemplate]] so that multiple templates can be
>>> inserted at page creation time. [[--intrigeri]]
>>>
>>>> I implemented such a warning using the formbuilder_setup hook.
>>>> --[[intrigeri]]
>>
>> And also, is there any way to start a translation of a page into a new
>> lanauge using the web interface?
>>
>>> When a new language is added to `po_slave_languages`, a rebuild is
>>> triggered, and all missing PO files are created and checked into
>>> VCS. An unpriviledged wiki user can not add a new language to
>>> `po_slave_languages`, though. One could think of adding the needed
>>> interface to translate a page into a yet-unsupported slave
>>> language, and this would automagically add this new language to
>>> `po_slave_languages`. It would probably be useful in some
>>> usecases, but I'm not comfortable with letting unpriviledged wiki
>>> users change the wiki configuration as a side effect of their
>>> actions; if this were to be implemented, special care would be
>>> needed. [[--intrigeri]]
>>>
>>>> Actually I meant into any of the currently supported languages.
>>>> I guess that if the template modification is made, it will list those
>>>> languages on the page, and if a translation to a language is missing,
>>>> the link will allow creating it?
>>>>
>>>>> Any translation page always exist for every supported slave
>>>>> language, even if no string at all have been translated yet.
>>>>> This implies the po plugin is especially friendly to people who
>>>>> prefer reading in their native language if available, but don't
>>>>> mind reading in English else.
>>>>>
>>>>> While I'm at it, there is a remaining issue that needs to be
>>>>> sorted out: how painful it could be for non-English speakers
>>>>> (assuming the master language is English) to be perfectly able
>>>>> to navigate between translation pages supposed to be written in
>>>>> their own language, when their translation level is most
>>>>> often low.
>>>>>
>>>>> (It is currently easy to display this status on the translation
>>>>> page itself, but then it's too late, and how frustrating to load
>>>>> a page just to realize it's actually not translated enough for
>>>>> you. The "other languages" loop also allows displaying this
>>>>> information, but it is generally not the primary
>>>>> navigation tool.)
>>>>>
>>>>> IMHO, this is actually a social problem (i.e. it's no use adding
>>>>> a language to the supported slave ones if you don't have the
>>>>> manpower to actually do the translations), that can't be fully
>>>>> solved by technical solutions, but I can think of some hacks
>>>>> that would limit the negative impact: a given translation's
>>>>> status (currently = percent translated) could be displayed next
>>>>> to the link that leads to it; a color code could as well be used
>>>>> ("just" a matter of adding a CSS id or class to the links,
>>>>> depending on this variable). As there is already work to be done
>>>>> to have the links text generation more customizable through
>>>>> plugins, I could do both at the same time if we consider this
>>>>> matter to be important enough. --[[intrigeri]]
>>>>>
>>>>>> The translation status in links is now implemented in my
>>>>>> `po`branch. It requires my `meta` branch changes to
>>>>>> work, though. I consider the latter to be mature enough to
>>>>>> be merged. --[[intrigeri]]

>> FWIW, I'm tracking your po branch in ikiwiki master git in the po
>> branch. One thing I'd like to try in there is setting up a translated
>> basewiki, which seems like it should be pretty easy to do, and would be
>> a great demo! --[[Joey]]
>>
>>> I have a complete translation of basewiki into danish, and am working with
>>> others on preparing one in german.  For a complete translated user
>>> experience, however, you will also need templates translated (there are a few
>>> translatable strings there too).  My not-yet-merged po4a Markdown improvements
>>> (see [bug#530574](http://bugs.debian.org/530574)) correctly handles multiple
>>> files in a single PO which might be relevant for template translation handling.
>>> --[[JonasSmedegaard]]
>>
>>> I've merged your changes into my own branch, and made great
>>> progress on the various todo items. Please note my repository
>>> location has changed a few days ago, my user page was updated
>>> accordingly, but I forgot to update this page at the same time.
>>> Hoping it's not too complicated to relocated an existing remote...
>>> (never done that, I'm a Git beginner as well as a Perl
>>> newbie) --[[intrigeri]]
>>>>
>>>> Just a matter of editing .git/config, thanks for the heads up.
>>>>>
>>>>> Joey, please have a look at my branch, your help would be really
>>>>> welcome for the security research, as I'm almost done with what
>>>>> I am able to do myself in this area. --[[intrigeri]]
>>>>>>
>>>>>> I came up with a patch for the WrapI18N issue --[[Joey]]

I've set this plugin development aside for a while. I will be back and
finish it at some point in the first quarter of 2009. --[[intrigeri]]

> Abstract: Joey, please have a look at my po and meta branches.
> 
> Detailed progress report:
> 
> * it seems the po branch in your repository has not been tracking my
>   own po branch for two months. any config issue?
> * all the plugin's todo items have been completed, robustness tests
>   done
> * I've finished the detailed security audit, and the fix for po4a
>   bugs has entered upstream CVS last week
> * I've merged your new `checkcontent` hook with the `cansave` hook
>   I previously introduced in my own branch; blogspam plugin updated
>   accordingly
> * the rename hook changes we discussed elsewhere are also part of my
>   branch
> * I've introduced two new hooks (`canremove` and `canrename`), not
>   a big deal; IMHO, they extend quite logically the plugin interface
> * as highlighted on [[bugs/pagetitle_function_does_not_respect_meta_titles]],
>   my `meta` branch contains a new feature that is really useful in a
>   translatable wiki
> 
> As a conclusion, I'm feeling that my branches are ready to be
> merged; only thing missing, I guess, are a bit of discussion and
> subsequent adjustments.
> 
> --[[intrigeri]]

> I've looked it over and updated my branch with some (untested)
> changes.
> 
>> I've merged your changes into my branch. Only one was buggy.
> 
> Sorry, I'd forgotten about your cansave hook.. sorry for the duplicate
> work there.
> 
> Reviewing the changes, mostly outside of `po.pm`, I have
> the following issues.
>  
> * renamepage to renamelink change would break the ikiwiki
>   3.x API, which I've promised not to do, so needs to be avoided
>   somehow. (Sorry, I guess I dropped the ball on not getting this
>   API change in before cutting 3.0..)
>> 
>> Fixed, see [[todo/need_global_renamepage_hook]].
>>
> * I don't understand the parentlinks code change and need to figure it
>   out. Can you explain what is going on there?
>> 
>> I'm calling `bestlink` there so that po's injected `bestlink` is
>> run. This way, the parent links of a page link to the parent page
>> version in the proper language, depending on the
>> `po_link_to=current` and `po_link_to=negotiated` settings.
>> Moreover, when using my meta branch enhancements plus meta title to
>> make pages titles translatable, this small patch is needed to get
>> the translated titles into parentlinks.
>> 
> * canrename's mix of positional and named parameters is way too
>   ugly to get into an ikiwiki API. Use named parameters
>   entirely. Also probably should just use named parameters
>   for canremove.
> * `skeleton.pm.example`'s canrename needs fixing to use either
>   the current or my suggested parameters.
>> 
>> Done.
>> 
> * I don't like the exporting of `%backlinks` and `$backlinks_calculated`
>   (the latter is exported but not used).
>> 
>> The commit message for 85f865b5d98e0122934d11e3f3eb6703e4f4c620
>> contains the rationale for this change. I guess I don't understand
>> the subtleties of `our` use, and perldoc does not help me a lot.
>> IIRC, I actually did not use `our` to "export" these variables, but
>> rather to have them shared between `Render.pm` uses.
>>
>>> My wording was unclear, I meant exposing. --[[Joey]]
>>>  
>>>> I guess I still don't know Perl's `our` enough to understand clearly.
>>>> No matter whether these variables are declared with `my` or `our`,
>>>> any plugin can `use IkiWiki::Render` and then access
>>>> `$IkiWiki::backlinks`, as already does e.g. the pagestat plugin.
>>>> So I guess your problem is not with letting plugins use these
>>>> variables, but with them being visible for every piece of
>>>> (possibly external) code called from `Render.pm`. Am I right?
>>>> If I understand clearly, using a brace block to lexically enclose
>>>> these two `our` declarations, alongside with the `calculate_backlinks`
>>>> and `backlinks` subs definitions, would be a proper solution, wouldn't
>>>> it? --[[intrigeri]]
>>>>
>>>>> No, %backlinks and the backlinks() function are not the same thing.
>>>>> The variable is lexically scoped; only accessible from inside
>>>>> `Render.pm` --[[Joey]] 
>>>> 
> * What is this `IkiWiki::nicepagetitle` and why are you
>   injecting it into that namespace when only your module uses it?
>   Actually, I can't even find a caller of it in your module.
>> 
>> I guess you should have a look to my `meta` branch and to
>> [[bugs/pagetitle_function_does_not_respect_meta_titles]] in order
>> to understand this :)
>>
>>> It would probably be good if I could merge this branch without 
>>> having to worry about also immediatly merging that one. --[[Joey]] 
>>> 
>>>> I removed all dependencies on my `meta` branch from the `po` one.
>>>> This implied removing the `po_translation_status_in_links` and
>>>> `po_strictly_refresh_backlinks` features, and every link text is now
>>>> displayed in the master language. I believe the removed features really
>>>> enhance user experience of a translatable wiki, that's why I was
>>>> initially supposing the `meta` branch would be merged first.
>>>> IMHO, we'll need to come back to this quite soon after `po` is merged.
>>>> --[[intrigeri]]
>>>>
>>>> Maybe you should keep those features in a meta-po branch?
>>>> I did a cursory review of your meta last night, have some issues with it, 
>>>> but this page isn't the place for a detailed review. --[[Joey]] 
>>>>
>>>>> Done. --[[intrigeri]]
>>> 
> * I'm very fearful of the `add_depends` in `postscan`. 
>   Does this make every page depend on every page that links
>   to it? Won't this absurdly bloat the dependency pagespecs
>   and slow everything down? And since nicepagetitle is given
>   as the reason for doing it, and nicepagetitle isn't used,
>   why do it?
>> 
>> As explained in the 85f865b5d98e0122934d11e3f3eb6703e4f4c620 log:
>> this feature hits performance a bit. Its cost was quite small in my
>> real-world use-cases (a few percents bigger refresh time), but
>> could be bigger in worst cases. When using the po plugin with my
>> meta branch changes (i.e. the `nicepagetitle` thing), and having
>> enabled the option to display translation status in links, this
>> maintains the translation status up-to-date in backlinks. Same when
>> using meta title to make the pages titles translatable. It does
>> help having a nice and consistent translated wiki, but as it can
>> also involve problems, I just turned it into an option.
>> 
>>> This has been completely removed for now due to the removal of
>>> the dependency on my `meta` branch. --[[intrigeri]]
>> 
> * The po4a Suggests should be versioned to the first version
>   that can be used safely, and that version documented in 
>   `plugins/po.mdwn`.
>>
>> Done.
>> 
>> --[[intrigeri]]
> 
> --[[Joey]] 

I reverted the `%backlinks` and `$backlinks_calculated` exposing.
The issue they were solving probably will arise again when I'll work
on my meta branch again (i.e. when the simplified po one is merged),
but the po thing is supposed to work without these ugly `our`.
Seems like it was the last unaddressed item from Joey's review, so I'm
daring a timid "please pull"... or rather, please review again :)
--[[intrigeri]]

> Ok, I've reviewed and merged into my own po branch. It's looking very
> mergeable.
> 
> * Is it worth trying to fix compatability with `indexpages`?
>> 
>> Supporting `usedirs` being enabled or disabled was already quite
>> hard IIRC, so supporting all four combinations of `usedirs` and
>> `indexpages` settings will probably be painful. I propose we forget
>> about it until someone reports he/she badly needs it, and then
>> we'll see what can be done.
>> 
> * Would it make sense to go ahead and modify `page.tmpl` to use
>   OTHERLANGUAGES and PERCENTTRANSLATED, instead of documenting how to modify it?
>> 
>> Done in my branch.
>> 
> * Would it be better to disable po support for pages that use unsupported
>   or poorly-supported markup languages?
> 
>> I prefer keeping it enabled, as:
>> 
>> * most wiki markups "almost work"
>> * when someone needs one of these to be fully supported, it's not
>>   that hard to add dedicated support for it to po4a; if it were
>>   disabled, I fear the ones who could do this would maybe think
>>   it's blandly impossible and give up.
>> 
 
> * What's the reasoning behind checking that the link plugin
>   is enabled? AFAICS, the same code in the scan hook should
>   also work when other link plugins like camelcase are used.
>> 
>> That's right, fixed.
>> 
> * In `pagetemplate` there is a comment that claims the code
>   relies on `genpage`, but I don't see how it does; it seems
>   to always add a discussion link?
>> 
>> It relies on IkiWiki::Render's `genpage` as this function sets the
>> `discussionlink` template param iff it considers a discussion link
>> should appear on the current page. That's why I'm testing
>> `$template->param('discussionlink')`.
>> 
>>> Maybe I was really wondering why it says it could lead to a broken
>>> link if the cgiurl is disabled. I think I see why now: Discussionlink
>>> will be set to a link to an existing disucssion page, even if cgi is
>>> disabled -- but there's no guarantee of a translated discussion page
>>> existing in that case. *However*, htmllink actually checks
>>> for this case, and will avoid generating a broken link so AFAICS, the
>>> comment is actually innacurate.. what will really happen in this case
>>> is discussionlink will be set to a non-link translation of
>>> "discussion". Also, I consider `$config{cgi}` and `%links` (etc)
>>> documented parts of the plugin interface, which won't change; po could
>>> rely on them to avoid this minor problem. --[[Joey]] 
>>>> 
>>>> Done in my branch. --[[intrigeri]]
>>>> 
>
> * Is there any real reason not to allow removing a translation?
>   I'm imagining a spammy translation, which an admin might not
>   be able to fix, but could remove.
>> 
>> On the other hand, allowing one to "remove" a translation would
>> probably lead to misunderstandings, as such a "removed" translation
>> page would appear back as soon as it is "removed" (with no strings
>> translated, though). I think an admin would be in a position to
>> delete the spammy `.po` file by hand using whatever VCS is in use.
>> Not that I'd really care, but I am slightly in favour of the way
>> it currently works.
>>
>>> That would definitly be confusing. It sounds to me like if we end up
>>> needing to allow web-based deletion of spammy translations, it will
>>> need improvements to the deletion UI to de-confuse that. It's fine to
>>> put that off until needed --[[Joey]] 
>> 
> * Re the meta title escaping issue worked around by `change`. 
>   I suppose this does not only affect meta, but other things
>   at scan time too. Also, handling it only on rebuild feels
>   suspicious -- a refresh could involve changes to multiple
>   pages and trigger the same problem, I think. Also, exposing
>   this rebuild to the user seems really ugly, not confidence inducing.
>   
>   So I wonder if there's a better way. Such as making po, at scan time,
>   re-run the scan hooks, passing them modified content (either converted
>   from po to mdwn or with the escaped stuff cheaply de-escaped). (Of
>   course the scan hook would need to avoid calling itself!)
> 
>   (This doesn't need to block the merge, but I hope it can be addressed
>   eventually..)
>  
> --[[Joey]] 
>> 
>> I'll think about it soon.
>> 
>> --[[intrigeri]]
>>
>>> Did you get a chance to? --[[Joey]] 

 * As discussed at [[todo/l10n]] the templates needs to be translatable too.  They
   should be treated properly by po4a using the markdown option - at least with my
   later patches in [bug#530574](http://bugs.debian.org/530574)) applied.

 * It seems to me that the po plugin (and possibly other parts of ikiwiki) wrongly
   uses gettext.  As I understand it, gettext (as used currently in ikiwiki) always
   lookup a single language, That might make sense for a single-language site, but
   multilingual sites should emit all strings targeted at the web output in each own
   language.

   So generally the system language (used for e.g. compile warnings) should be separated
   from both master language and slave languages.

   Preferrably the gettext subroutine could be extended to pass locale as optional
   secondary parameter overriding the default locale (for messages like "N/A" as
   percentage in po plugin).  Alternatively (with above mentioned template support)
   all such strings could be externalized as templates that can then be localized.

# Robustness tests

### Enabling/disabling the plugin

* enabling the plugin with `po_translatable_pages` set to blacklist: **OK**
* enabling the plugin with `po_translatable_pages` set to whitelist: **OK**
* enabling the plugin without `po_translatable_pages` set: **OK**
* disabling the plugin: **OK**

### Changing the plugin config

* adding existing pages to `po_translatable_pages`: **OK**
* removing existing pages from `po_translatable_pages`: **OK**
* adding a language to `po_slave_languages`: **OK**
* removing a language from `po_slave_languages`: **OK**
* changing `po_master_language`: **OK**
* replacing `po_master_language` with a language previously part of
  `po_slave_languages`: needs two rebuilds, but **OK** (this is quite
  a perverse test actually)

### Creating/deleting/renaming pages

All cases of master/slave page creation/deletion/rename, both via RCS
and via CGI, have been tested.

### Misc

* general test with `usedirs` disabled: **OK**
* general test with `indexpages` enabled: **not OK**
* general test with `po_link_to=default` with `userdirs` enabled: **OK**
* general test with `po_link_to=default` with `userdirs` disabled: **OK**