Skip to content
This repository has been archived by the owner on Apr 1, 2019. It is now read-only.

locale/newTab.js should be an JSON file #114

Closed
marcoscaceres opened this issue Oct 21, 2015 · 4 comments
Closed

locale/newTab.js should be an JSON file #114

marcoscaceres opened this issue Oct 21, 2015 · 4 comments

Comments

@marcoscaceres
Copy link
Contributor

we should load newTab.js strings as needed, using fetch as XML.

@marcoscaceres marcoscaceres changed the title locale/newTab.js should be a JSON file locale/newTab.js should be an XML file Nov 2, 2015
@marcoscaceres
Copy link
Contributor Author

Loosely depends on:
whatwg/fetch#151

@marcoscaceres marcoscaceres added this to the V2 milestone Nov 2, 2015
@marcoscaceres
Copy link
Contributor Author

Can do:

async function fetchXML(url){
  var response = await fetch(url);
  var text = await response.text(); 
  return (new DOMParser()).parseFromString(text, "text/xml");
}

@oyiptong
Copy link
Contributor

oyiptong commented Nov 2, 2015

Any reason why we want locale/newTab.js as XML?
We can make it JSON.

If we re-use the XML as-is currently, there is a need for 2 requests: the xml and the DTD which actually contains the text:

http://mxr.mozilla.org/l10n-central/source/en-GB/browser/chrome/browser/newTab.dtd
http://mxr.mozilla.org/l10n-central/source/en-GB/browser/chrome/browser/newTab.properties

There is also no locale XML for en-US in l10n-central.

How about we provide one JSON file per locale, generated as a build step?

@marcoscaceres
Copy link
Contributor Author

(Note, this is a v2 thing)

On Nov 3, 2015, at 3:14 AM, Olivier Yiptong notifications@github.com wrote:

Any reason why we want locale/newTab.js as XML?
We can make it JSON

That would be great - just need to remove the comments. Right now, anyone localizing could add arbitrary code to the locales file, which makes me a bit uncomfortable. This could break things, or worse, obviously.
If we re-use the XML as-is currently, there is a need for 2 requests: the xml and the DTD which actually contains the text:

http://mxr.mozilla.org/l10n-central/source/en-GB/browser/chrome/browser/newTab.dtd
http://mxr.mozilla.org/l10n-central/source/en-GB/browser/chrome/browser/newTab.properties

Yeah, the currently used format is pretty awful.

There is also no locale XML for en-US in l10n-central.

How about we provide one JSON file per locale, generated as a build step?

That would be ideal:)


Reply to this email directly or view it on GitHub.

@marcoscaceres marcoscaceres changed the title locale/newTab.js should be an XML file locale/newTab.js should be an JSON file Nov 2, 2015
@marcoscaceres marcoscaceres removed this from the V2 milestone Nov 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants