907146 - Update browser/locales/filter.py for Firefox Metro's search engines (region.properties)

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Build Config, defect)

Description

[Francesco Lodolo [:flod]]

p12n information for Firefox Metro are now stored inside a separate region.properties file in /browser/metro/chrome/browser.properties
http://hg.mozilla.org/l10n-central/it/rev/f044d6e1f562

When Metro's localization was dropped some changes to filter.py remained (bug 769490). I think the only part missing is to adapt for a different number of search engines between en-US and other locales

Comment 1

[Francesco Lodolo [:flod]]

If I read correctly how that file works, would it be enough to add this after the first check (if mod not in) and before it exits for everything except browser or spellcheck?

  if mod == "browser/metro" and path == "chrome/region.properties":
    # Firefox Metro: ignore search engines order
    return not (re.match(r"browser\.search\.order\.[1-9]", entity))

Problem is that I don't really understand how this file is used or how to test it.

Comment 2

[Axel Hecht [:Pike]]

Attachment: Rewrite filter.py to be easier to grok, and support metro. drop help images

I'll just take this bug.

I'm rewriting filter.py a good deal here, and replacing False with "ignore", and True with "error". The file just makes a metric ton of s*** more sense that way.

I'm also using conditional expressions freely. Python 2.5 is OK, I hope.

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 798848 [details] [diff] [review]
Rewrite filter.py to be easier to grok, and support metro. drop help images

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

Looks good to me, tried also running some tests against both functions getting the same results.

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 798848 [details] [diff] [review]
Rewrite filter.py to be easier to grok, and support metro. drop help images

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

I have no clue what this code does. Can you find another reviewer who does?

Comment 5

[Axel Hecht [:Pike]]

I think r=sparky is the next best solution, given that this part of the l10n setup is me, myself and I, really.

Wanna wave that?

Comment 6

[Axel Hecht [:Pike]]

Talked this through with gavin over irc, he doesn't require more reviews on this patch.

Thus landed with https://hg.mozilla.org/integration/mozilla-inbound/rev/7dd0ddad0460

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7dd0ddad0460