1306602 - FTL parser does not handle Windows end of line ('\r\n')

Status: UNCONFIRMED

(L20n :: JS Library, defect)

Description

[Francesc Baeta]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160922113459

Steps to reproduce:

l20n.js version: 4.x (master)

Have a FTL file with windows style end of lines '\r\n'.
Example:
refresh-domain =
	[html/title] Rafraîchir le domaine

The translation was used for a button title.
<button data-l10n-id="refresh-domain"><span><i class="my-icon"></i></span></button>


Actual results:

The "innerHTML" of the button was removed (<span><i class="my-icon"></i></span>).


Expected results:

The "innerHTML" must be kept.

Comment 1

[Francesc Baeta]

The problem is that parser result is:
{
	"traits": [{
		"key": {
			"type": "kw",
			"ns": "html",
			"name": "title"
		},
		"val": "Rafraîchir le domaine"
	}],
	"val": "\r"
}

And should be:
{
	"traits": [{
		"key": {
			"type": "kw",
			"ns": "html",
			"name": "title"
		},
		"val": "Rafraîchir le domaine"
	}]
}"

Comment 2

[Francesc Baeta]

Attachment: Pull Request

Comment 3

[Staś Małolepszy :stas]

Comment on attachment 8796547 [details] [review]
Pull Request

Thanks for the detailed report and the fix!  Zibi, can you take a look at this please?

Comment 4

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

This is tricky.

So, this patch unfortunately is against entries parser which is performance critical. What's more, it affects the very hot path of code.

The current patch regresses performance by ~8%:

parseFTL: 
  mean:   12108.77 (+1%)
  stdev:  1024.02
  sample: 30
parseFTLEntries: 
  mean:   4869.97 (+8%)
  stdev:  434.75
  sample: 30
format: 
  mean:   1280.3 (-1%)
  stdev:  124.94
  sample: 30

It's going to be hard to accept the perf cost in the current state.
I have to think more about how can we solve it without paying this.

Comment 5

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

Comment on attachment 8796547 [details] [review]
Pull Request

sorry, that was for a different patch. This one is still ?

Comment 6

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

Comment on attachment 8796547 [details] [review]
Pull Request

dropping r? - we shifted work to Project Fluent and that parser should handle Windows newlines.