Our fairly dummy l10n-merge in FirefoxOS is currently merging whole entities irrelevant of their fields. That causes problems when the obsolete entity in ab-CD contains different fields than new en-US. A solution would be to look into entity and copy only the fields (value/attributes) that are in source locale.
So, what this patch does is that it changes how we do our naive l10n-merge. With it, we will look into the top-level structure of each entity and verify if it has the same fields. If the structure of the entity is different, we'll pick the source entity. That also matches closer to how it will work in l20n where we will have the concept of malformed entity (entity with public fields not matching source entity). For now, it will fix the problem of obsolete entities with values leaking into result locales and causing UI regressions.
Comment on attachment 8513942 [details] [review] pull request I commented in the PR, although pretty please attach textual diffs in Bugzilla in the future. r=me with comments addressed.
Attachment #8513942 - Flags: review?(stas) → review+
Comment on attachment 8513942 [details] [review] pull request Thanks Stas for the review!
Commit: https://github.com/zbraniecki/gaia/commit/d1984db195e4fdd681819ea033c4a57d5afcd25b Merge: https://github.com/mozilla-b2g/gaia/commit/3b3179b06cd85b3548688b67dde050fe20b2753a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
I landed this in the l20n.js repo as well: https://github.com/l20n/l20n.js/commit/c81d1981d74708e5d8e56c825fc996c6b777304b Zibi, I thought you were going to land the version with keys1.every() ?
(In reply to Staś Małolepszy :stas from comment #5) > I landed this in the l20n.js repo as well: > > https://github.com/l20n/l20n.js/commit/ > c81d1981d74708e5d8e56c825fc996c6b777304b Oh, thanks! > Zibi, I thought you were going to land the version with keys1.every() ? No, since we decided not to go for two loops, but rather length match plus one loop I don't think that an external function makes sense and inline loop does not inflate the stack.
I agreed with you not to do (keys1.every && keys2.every). However, I thought we said keys1.every was okay. It's one loop, a much cleaner functional approach and is only one line rather than 3 which leak the implementation of for-loop.
Anyways, this had my r+, so it's okay, I just wanted to clarify my position and my preference on the style. I do think keys1.every is easier to read and we're not even remotely close to care about the call stack here.
Yeah, I'm afraid that it's a difference in our preferences - imperative vs. declarative. Yours seems to be driven by code readability, mine on CPU cycles, performance , and memory.  http://jsperf.com/every-vs-loop222
You need to log in before you can comment on or make changes to this bug.