Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixing plural and properties other than textContent #75

Open
Phyks opened this issue Oct 14, 2015 · 9 comments
Open

Mixing plural and properties other than textContent #75

Phyks opened this issue Oct 14, 2015 · 9 comments

Comments

@Phyks
Copy link
Contributor

Phyks commented Oct 14, 2015

Hi,

I am trying to mix plural and properties other than textContent, such as innerHTML.

My properties file contains:

available_bikes.innerHTML = {[ plural(n) ]}
available_bikes[zero].innerHTML = available<br/>bike
available_bikes[one].innerHTML = available<br/>bike
available_bikes[other].innerHTML = available<br/>bikes

And my HTML file contains:

<span data-l10n-id="available_bikes" data-l10n-args='{ "n": "42" }'>vélos<br/>disponibles</span>

Which, as far as I understand the syntax and the way the code works, should be working. But it does not. It displays {[ plural(n) ]}, which is unexpected. If I edit the HTML file to look like this:

                    <span data-l10n-id="available_bikes[other]">vélos<br/>disponibles</span>

it works.

Do you know if I am doing anything wrong or if this is a bug?

Thanks

@Phyks
Copy link
Contributor Author

Phyks commented Nov 15, 2015

Anyone having a solution for this issue? Thanks!

@Rob--W
Copy link
Collaborator

Rob--W commented Nov 15, 2015

Try stepping through with a debugger to see what goes wrong.

I suggest to put a breakpoint at the start of gMacros.plural.

@Phyks
Copy link
Contributor Author

Phyks commented Nov 16, 2015

@Rob--W It is due to https://github.com/fabi1cazenave/webL10n/blob/master/l10n.js#L811 which bypasses evaluation for non-textual properties such as innerHTML.

Is there any reason to have this line of code?

@Rob--W
Copy link
Collaborator

Rob--W commented Nov 16, 2015

@Phyks I guess that it's just not implemented due to the lack of need / time. @fabi1cazenave can give a more authorative answer to why that line of code is there.

@Phyks
Copy link
Contributor Author

Phyks commented Nov 16, 2015

@Rob--W @fabi1cazenave Ok, I just removed the faulty line and it works for my use case. I guess it can break something else, and if you have feedback about it, I am interested in them.

In the meantime, I guess this is just the solution for anyone experiencing the same issue :)

@Phyks
Copy link
Contributor Author

Phyks commented Mar 30, 2016

Any advice on it? Interested in a PR?

@Rob--W
Copy link
Collaborator

Rob--W commented Mar 30, 2016

I don't see any immediate issues with allowing plurals for non-textContent properties. If @fabi1cazenave has no objections, I would accept a PR.

@Phyks
Copy link
Contributor Author

Phyks commented May 30, 2016

Bump

@Rob--W
Copy link
Collaborator

Rob--W commented May 30, 2016

I think that you'd better shoot him a mail if you want feedback from @fabi1cazenave .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants