Allow reordering HTML elements in translations

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: stas, Assigned: stas)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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]
Can't it be done using `insertBefore`?
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
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
Attachment #813869 - Flags: review?(gandalf)
Comment on attachment 813869 [details] [diff] [review]
Allow reordering

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

sweet work Stas!
Attachment #813869 - Flags: review?(gandalf) → review+
Assignee: nobody → stas
(Assignee)

Comment 5

6 years ago
Comment on attachment 813869 [details] [diff] [review]
Allow reordering

Michał, would you mind taking a look and seeing if this still works with Angular?
Attachment #813869 - Flags: feedback?(m.goleb+mozilla)
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
(Assignee)

Comment 7

6 years ago
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).
Flags: needinfo?(m.goleb+mozilla)
(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. ;)
Flags: needinfo?(m.goleb+mozilla)
Apart from these errors, it seems to work fine.
@stas Ah, sorry, those errors are already present on master, some other changes had to introduce them. :/
(Assignee)

Comment 11

6 years ago
(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.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Attachment #813869 - Flags: feedback?(m.goleb+mozilla) → feedback+
You need to log in before you can comment on or make changes to this bug.