922576 - Allow reordering HTML elements in translations

Status: RESOLVED FIXED

(L20n :: HTML Bindings, defect)

Description

[Staś Małolepszy :stas]

The work in bug 921169 made it impossible (or risky) to reorder elements in the translations.  We should consider allowing (from easiest to most difficult):

 1. reordering elements of different type: input a → a input,
 2. reordering elements of same type with an id: a#one a#two → a#two a#one (this introduces a dependency on developers who need to add ids),
 3. reordering elements of same type without additional work from developers: a a → a#l10n-path=a[2] a#l10n-path=a[1]

Comment 1

[Michał Gołębiowski-Owczarek [:mgol]]

Can't it be done using `insertBefore`?

Comment 2

[Staś Małolepszy :stas]

We already use insertBefore.  The problem is that I want to identify elements that need not be replaced, but also insert new elements.  Right now, I do this sequentially:  start from the first child in the source, find its equivalent in the translation, take all nodes up that equivalent from the translation and move them into source.  If source has two elements (input a), I'll start with input, find it in the translation (where the order is reversed: a input), take all nodes preceding input (a) and move them into source.  Then wehn I move on to 'a' in source, I don't find it in the translation anymore.

So doing this sequentially is limiting.

An alternative would be to sanitize translation's DOM first and then use translation.replaceChild to move the source element from source DOM to translation overlay it there, and then move the whole translation back into source.

Comment 3

[Staś Małolepszy :stas]

Attachment: Allow reordering

This patch implements the approach from comment 2 in order to allow the first 
type of the reordering mentioned in comment 0.  I should have done this like 
this from the beginning, it's much simpler and just as fast.

Settings, l20n-master, languages_dev, 30 iterations:

master: 1139
patch: 1143

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 813869 [details] [diff] [review]
Allow reordering

Review of attachment 813869 [details] [diff] [review]:
-----------------------------------------------------------------

sweet work Stas!

Comment 5

[Staś Małolepszy :stas]

Comment on attachment 813869 [details] [diff] [review]
Allow reordering

Michał, would you mind taking a look and seeing if this still works with Angular?

Comment 6

[Michał Gołębiowski-Owczarek [:mgol]]

Stas: your patch generates a massive amount of errors to the console in Polona. There are two kinds of errors, here are tracebacks: http://pastebin.com/z5WSMMQq

Comment 7

[Staś Małolepszy :stas]

Michał:  are these really related to my patch?  I think they might be caused by the bug we discussed on IRC last week about _source being null instead of '' when the resource download fails (I filed bug 924071 for this).

Comment 8

[Michał Gołębiowski-Owczarek [:mgol]]

(In reply to Staś Małolepszy :stas from comment #7)
> Michał:  are these really related to my patch?  I think they might be caused
> by the bug we discussed on IRC last week about _source being null instead of
> '' when the resource download fails (I filed bug 924071 for this).

That might be true, your patch might have just made these bugs more explicit. However, that's what I see after applying your patch, I'm not saying it's your fault, I only said what happened. ;)

Comment 9

[Michał Gołębiowski-Owczarek [:mgol]]

Apart from these errors, it seems to work fine.

Comment 10

[Michał Gołębiowski-Owczarek [:mgol]]

@stas Ah, sorry, those errors are already present on master, some other changes had to introduce them. :/

Comment 11

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas from comment #0)

>  1. reordering elements of different type: input a → a input,

Fixed in https://github.com/l20n/l20n.js/commit/97ecf63377a789aeaff2f08b3e366fbbc47c4fd6

>  2. reordering elements of same type with an id: a#one a#two → a#two a#one
> (this introduces a dependency on developers who need to add ids),
>  3. reordering elements of same type without additional work from
> developers: a a → a#l10n-path=a[2] a#l10n-path=a[1]

If we get to implementing #2 and #3, we should file new bugs.