[Unicode]   Common Locale Data Repository : Bug Tracking Home | Site Map | Search
 
Modify

CLDR Ticket #10289(closed: fixed)

Opened 18 months ago

Last modified 4 weeks ago

Default Coverage level does not reflect info in Locales.txt

Reported by: kristi Owned by: tbishop
Component: survey-backend Data Locale:
Phase: dsub Review: mark
Weeks: Data Xpath:
Xref:

Description (last modified by tbishop) (diff)

When I log in to the Survey tool using a vetter account for a locale that has a Basic setting in Locales.txt, the Coverage level remains as Modern.

I thought the coverage level in locales.txt is used for the ST vetter view as well as the reports.

Attachments

Change History

comment:1 Changed 18 months ago by mark

  • Owner changed from anybody to mark
  • Priority changed from assess to critical
  • Status changed from new to accepted
  • Milestone changed from UNSCH to 32

comment:2 Changed 18 months ago by mark

Verified that this is a problem.

Logged into smoketest as (fake) user on http://cldr-smoke.unicode.org/smoketest/createAndLogin.jsp

Spec. Org: Microsoft
User Level: 5 (VETTER)
Language: chr (first basic locale on Locales.txt)

Clicked on Cherokee, got Modern Coverage.

comment:3 Changed 18 months ago by mark

Same happened with Google user, using 'chr' which is moderate for Google.

comment:4 Changed 18 months ago by mark

Notes while forensing.

Inspected source, to find:

<li class="dropdown" id="title-coverage" style="">

<a href="#" class="dropdown-toggle" data-toggle="dropdown">Coverage: <span id="coverage-info">Modern (Default)</span></a>
<ul class="dropdown-menu">
<li><a class="coverage-list" data-value="auto" href="#">Auto</a></li><li><a class="coverage-list" data-value="core" href="#">Core</a></li><li><a class="coverage-list" data-value="basic" href="#">Basic</a></li><li><a class="coverage-list" data-value="moderate" href="#">Moderate</a></li><li><a class="coverage-list" data-value="modern" href="#">Modern</a></li><li><a class="coverage-list" data-value="comprehensive" href="#">Comprehensive</a></li></ul>

</li>

The menu is in v.jsp, but the contents must be determined elsewhere.

<li class="dropdown" id='title-coverage' style='display:none'>

<a href="#" class="dropdown-toggle" data-toggle="dropdown">Coverage: <span id="coverage-info"></span></a>
<ul class="dropdown-menu">
</ul>

</li>



Searches for ids 'coverage-info' and 'title-coverage' get nothing.

<span id="coverage-info">Modern (Default)</span>

File filter was not lenient, using *.html, *.java, *.js, *.jsp, *.txt
Found in survey.js

comment:5 Changed 15 months ago by mark

  • Milestone changed from 32 to 33

comment:6 Changed 9 months ago by mark

  • Owner changed from mark to backend

comment:7 Changed 9 months ago by mark

  • Milestone changed from 33 to 34

comment:8 Changed 9 months ago by kristi

Fix the coverage setting so that when going to a new locale, it defaults to the organization setting

comment:9 Changed 9 months ago by kristi

  • Keywords STGen added

comment:10 Changed 7 weeks ago by pedberg

  • Milestone changed from 34 to 35

comment:11 Changed 7 weeks ago by tbishop

  • Owner changed from backend to tbishop
  • type changed from unknown to survey
  • Component changed from unknown to survey

Per today's meeting I've reassigned this to myself.

comment:12 Changed 6 weeks ago by tbishop

  • Description modified (diff)
  • Summary changed from Default Coverage level does not reflect info in Locale.txt to Default Coverage level does not reflect info in Locales.txt

comment:13 Changed 6 weeks ago by tbishop

Stepping through the code, when I log in as a new user, a Vetter for locale "chr", this function is called:

    /**
     * Get the options for the given WebContext, or, if the context is null, get the
     * options for the given CookieSession and CLDRLocale
     *
     * @param ctx
     * @param session
     * @param locale
     * @return the CheckCLDR.Options object
     */
    public static CheckCLDR.Options getOptions(WebContext ctx, CookieSession session, CLDRLocale locale) {
        CheckCLDR.Options options;
        if (ctx != null) {
            options = (ctx.getOptionsMap());
        } else {
            final String def = CookieSession.sm
                .getListSetting(session.settings(), SurveyMain.PREF_COVLEV,
                    WebContext.PREF_COVLEV_LIST, false);

            final String org = session.getEffectiveCoverageLevel(locale.toString());

            options = new Options(locale, SurveyMain.getTestPhase(), def, org);
        }
        return options;
    }

It takes the first branch, for getOptionsMap, not the "else" branch for getEffectiveCoverageLevel:

    /**
     * Append the WebContext Options map to the specified map
     *
     * @return the map
     */
    public CheckCLDR.Options getOptionsMap() {
        String def = getRequiredCoverageLevel();
        String org = getEffectiveCoverageLevel();

        return new CheckCLDR.Options(getLocale(), SurveyMain.getTestPhase(), def, org);
    }

There we get def = "default" and org = "modern".

comment:14 Changed 6 weeks ago by tbishop

Locales.txt is read here in StandardCodes.java:

   private void loadPlatformLocaleStatus() {
        LocaleIDParser parser = new LocaleIDParser();
        platform_locale_level = new EnumMap<Organization, Map<String, Level>>(Organization.class);
        SupplementalDataInfo sd = SupplementalDataInfo.getInstance();
        Set<String> defaultContentLocales = sd.getDefaultContentLocales();
        String line;
        try {
            BufferedReader lstreg = CldrUtility.getUTF8Data("Locales.txt");

Locales.txt contains three lines with "chr":

Google ; chr ; basic ; Cherokee
Cherokee ; chr ; modern ; Cherokee
Microsoft ; chr ; basic

However, the temporary user I created happened to belong to Mozilla, and isCoverageOrganization here is false:

    /**
     * Get the coverage level for my organization (if I have one)
     * @param locale
     * @return
     */
    public String getOrgCoverageLevel(String locale) {
        String level;
        String myOrg = getUserOrg();
        if ((myOrg == null) || !WebContext.isCoverageOrganization(myOrg)) {
            level = WebContext.COVLEV_DEFAULT_RECOMMENDED_STRING;
        } else {
            org.unicode.cldr.util.Level l = StandardCodes.make().getLocaleCoverageLevel(myOrg, locale);
            if (l == Level.UNDETERMINED) {
                l = WebContext.COVLEVEL_DEFAULT_RECOMMENDED;
            }
            level = l.toString();
        }
        return level;
    }

Do it again with new Microsoft vetter, still isCoverageOrganization returns false.

    /**
     * Is it an organization that participates in coverage?
     *
     * @param org
     * @return
     */
    public static boolean isCoverageOrganization(String org) {
        return (org != null && StandardCodes.make().getLocaleCoverageOrganizations().contains(org));
    }

There, getLocaleCoverageOrganizations returns:

[adlam, adobe, afghan_csa, afghan_mcit, afrigen, apple, bangladesh, bangor_univ, bhutan, breton, cherokee, cldr, facebook, gaeilge, georgia_isi, gnome, google, guest, ibm, india, iran_hci, kendra, kotoistus, lakota_lc, lao_dpt, longnow, microsoft, mozilla, netflix, openinstitute, openoffice_org, oracle, pakistan, rumantscha, sil, srilanka, surveytool, welsh_lc, wikimedia, yahoo]

The function contains still returns false. Maybe due to the difference in capitalization, "microsoft" versus "Microsoft"? No, that's not quite it. Each member of the set returned by getLocaleCoverageOrganizations is an Organization, not a String. It's confusing since there are two functions named getLocaleCoverageOrganizations. One returns a set of Organizations, the other returns an array of Strings:

    /**
     * Get a list of all locale types
     *
     * @return a list of locale types
     * @see StandardCodes#getLocaleCoverageOrganizations()
     */
    static String[] getLocaleCoverageOrganizations() {
        Set<Organization> set = StandardCodes.make().getLocaleCoverageOrganizations();
        Organization[] orgArray = set.toArray(new Organization[0]);
        String[] stringArray = new String[orgArray.length];
        int i = 0;
        for (Organization org : orgArray) {
            stringArray[i++] = org.toString();
        }
        return stringArray;
        // There was ArrayStoreException here:
        // return StandardCodes.make().getLocaleCoverageOrganizations().toArray(new String[0]);
    }
Last edited 6 weeks ago by tbishop (previous) (diff)

comment:15 Changed 6 weeks ago by tbishop

This change appears to solve the bug:

Index: tools/cldr-apps/src/org/unicode/cldr/web/WebContext.java
...
     public static boolean isCoverageOrganization(String org) {
-        return (org != null && StandardCodes.make().getLocaleCoverageOrganizations().contains(org));
+        return (org != null && StandardCodes.make().getLocaleCoverageOrganizations().contains(Organization.fromString(org)));
     }

comment:16 Changed 5 weeks ago by tbishop

I added a unit test in TestMisc.java:

    /**
     * Test that the function WebContext.isCoverageOrganization returns
     * true for "Microsoft" and "microsoft", and false for "FakeOrgName".
     *
     * Reference: https://unicode.org/cldr/trac/ticket/10289
     */
    public void TestIsCoverageOrganization() {
        String orgName = "Microsoft";
        try {
            if (!WebContext.isCoverageOrganization(orgName)) {
                errln("❌ isCoverageOrganization(" + orgName + ") false, expected true.");
                return;
            }
            orgName = orgName.toLowerCase();
            if (!WebContext.isCoverageOrganization(orgName)) {
                errln("❌ isCoverageOrganization(" + orgName + ") false, expected true.");
                return;
            }
            orgName = "FakeOrgName";
            if (WebContext.isCoverageOrganization(orgName)) {
                errln("❌ isCoverageOrganization(" + orgName + ") true, expected false.");
                return;
            }
        } catch (Exception e) {
            errln("❌ isCoverageOrganization(" + orgName + "). Unexpected exception: " + e.toString() + " - " + e.getMessage());
            return;
        }
        System.out.println("✅");
    }

As intended, that test fails for the old version of isCoverageOrganization, and passes for the new version which is:

    /**
     * Is it an organization that participates in coverage?
     *
     * @param org the name of the organization
     * @return true if the organization participates in coverage, else false
     */
    public static boolean isCoverageOrganization(String org) {
        return (org != null && StandardCodes.make().getLocaleCoverageOrganizations().contains(Organization.fromString(org)));
    }
Last edited 5 weeks ago by tbishop (previous) (diff)

comment:17 Changed 5 weeks ago by tbishop

I committed to trunk with changeset [14531]. Tested on SmokeTest, logged in as Microsoft vetter for chr (Cherokee), got Basic coverage. Success!

Last edited 5 weeks ago by tbishop (previous) (diff)

comment:18 Changed 5 weeks ago by tbishop

  • Status changed from accepted to reviewing
  • Review set to mark

comment:19 Changed 5 weeks ago by mark

  • Status changed from reviewing to reviewfeedback
  • Milestone changed from 35 to 34

Resetting to v34, since checked in for this release.

Simpler and more reliable fix is:

return getLocaleCoverageOrganizationStrings.contains(org);

More reliable because won't get exception if org isn't an Organization.

comment:20 Changed 5 weeks ago by tbishop

isCoverageOrganization actually didn't throw an exception when called with "FakeOrgName" in the unit test. Anyway, I changed from

return (org != null && StandardCodes.make().getLocaleCoverageOrganizations().contains(Organization.fromString(org)));

to

return (org != null && StandardCodes.make().getLocaleCoverageOrganizationStrings().contains(org));

then the unit test failed: TestIsCoverageOrganization(TestMisc.java:63) ❌ isCoverageOrganization(Microsoft) false, expected true.

The string needs to be lowercase to be found in getLocaleCoverageOrganizationStrings. I added toLowerCase as follows:

return (org != null && StandardCodes.make().getLocaleCoverageOrganizationStrings().contains(org.toLowerCase()));

Now, "<< ALL TESTS PASSED >>". Ran ST on localhost, logged in as Microsoft vetter for chr, coverage is Basic. Success.

comment:21 Changed 5 weeks ago by tbishop

I committed changeset [14538] to trunk. Will test on SmokeTest.

comment:22 Changed 5 weeks ago by tbishop

Tested OK on SmokeTest. Logged in as Microsoft vetter for chr (Cherokee). Coverage is automatically be Basic.

comment:23 Changed 5 weeks ago by tbishop

  • Status changed from reviewfeedback to reviewing

comment:24 Changed 4 weeks ago by mark

  • Status changed from reviewing to closed
  • Resolution set to fixed
View

Add a comment

Modify Ticket

Action
as closed
Next status will be 'new'
Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.