Opened 21 months ago

Closed 3 months ago

Last modified 2 months ago

#3697 closed change (fixed)

Fall back to i18n.getUILanguage if @ui_locale isn't supported

Reported by: sebastian Assignee: oleksandr
Priority: P2 Milestone: Adblock-Plus-1.13.4-for-Chrome-Opera
Module: Platform Keywords:
Cc: scottlow, kzar, Ross Blocked By: #4084
Blocking: Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29338190/

Description (last modified by kzar)

Background

Much of the user interface in Adblock Plus relies on knowing which language the user is using, and the reading direction for it. Currently we use the getMessage API in combination with some special message IDs:

  • chrome.i18n.getMessage("@ui_locale") returns the language of the browser's user interface.
  • chrome.i18n.getMessage("@bidi_dir") returns the respective reading direction of that language.

The problem is that Microsoft Edge does not yet support those message IDs yet.

What to change

  • Use i18n.getUILanguage instead of i18n.getMessage("@ui_locale").
  • If i18n.getMessage("@bidi_dir") returns "" (empty string) fall back to the same logic we had on Safari to determine the reading direction of the UI locale, matching the locale against a list of known right-to-left languages.
  • Update the adblockplusui dependency to revision 104a61b1949d.

Note: The adblockplusui dependency update includes many other changes. They mostly relate to the new options page and the copyright headers, but there is one small change to how the links on the first run page are setup.

Hints for testers

  • Ensure that the correct filter list is selected by default.
  • Ensure that the correct UI language is selected by default.
  • Check that right-to-left detection works correctly for languages that require that (for example Arabic).
  • Ensure that the links on the first run page still all work.

Change History (17)

comment:1 Changed 21 months ago by sebastian

  • Summary changed from Fall back to i18n.getUILanguage if @ui_locale isn't supported. to Fall back to i18n.getUILanguage if @ui_locale isn't supported

comment:2 Changed 21 months ago by oleksandr

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed 19 months ago by sebastian

  • Cc scottlow added

comment:4 Changed 18 months ago by sebastian

  • Keywords edge removed
  • Platform changed from Unknown / Cross platform to Edge

comment:5 Changed 18 months ago by sebastian

  • Sensitive unset

comment:6 Changed 3 months ago by sebastian

  • Blocked By 4084 added

comment:7 Changed 3 months ago by sebastian

  • Description modified (diff)

comment:8 Changed 3 months ago by oleksandr

  • Description modified (diff)

comment:9 Changed 3 months ago by oleksandr

  • Description modified (diff)

comment:10 Changed 3 months ago by kzar

  • Description modified (diff)

comment:11 Changed 3 months ago by kzar

  • Cc kzar added
  • Owner set to oleksandr

comment:12 Changed 3 months ago by abpbot

comment:13 Changed 3 months ago by oleksandr

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

comment:14 Changed 3 months ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:15 follow-up: Changed 2 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. The correct filter lists are added. First run page links work. Language and right-to-left work as expected.

ABP 1.13.13.1838
Chrome 49 / 60 / Windows 7
Opera 36 / 46 / Windows 7

ABP 0.9.11.1842
Edge 40 / Windows 10

Last edited 2 months ago by Ross (previous) (diff)

comment:16 in reply to: ↑ 15 ; follow-up: Changed 2 months ago by sebastian

  • Cc Ross added

Replying to Ross:

Edge 40 / Windows 7

This seems to be wrong, Microsoft Edge doesn't exist before Windows 10.

comment:17 in reply to: ↑ 16 Changed 2 months ago by Ross

Replying to sebastian:

Replying to Ross:

Edge 40 / Windows 7

This seems to be wrong, Microsoft Edge doesn't exist before Windows 10.

Yep. I fixed it.

Note: See TracTickets for help on using tickets.