Issue 4084 - Do not use @@bidi_dir directly in adblockplusui

Issue 4084 - Do not use @@bidi_dir directly in adblockplusui

[Sebastian Noack, 2016-05-29 14:05:08]

https://codereview.adblockplus.org/29345232/diff/29345233/messageResponder.js
File messageResponder.js (right):

https://codereview.adblockplus.org/29345232/diff/29345233/messageResponder.js...
messageResponder.js:184: bidiDir = Utils.readingDirection;
You have to change the mock implementation in adblockplusui/background.js as
well.

[Oleksandr, 2016-05-30 14:00:25]

I guess just mocking the readingDirection is enough.

[Sebastian Noack, 2016-05-30 14:16:57]

https://codereview.adblockplus.org/29345232/diff/29345366/messageResponder.js
File messageResponder.js (right):

https://codereview.adblockplus.org/29345232/diff/29345366/messageResponder.js...
messageResponder.js:184: bidiDir = Utils.readingDirection;
I guess we should move the logic for @@bidi_dir and @@ui_locale from
ext/common.js here. If those aren't supported by ext.chrome.getMessage() on all
platforms, the mock implementation shouldn't support it either, otherwise
somebody might start using it without noticing that it won't work on Edge.

[Oleksandr, 2016-05-30 17:01:33]

It is a bit clumsy without getMessage, but I guess you do have a point. Now,
however, one might start using ext.i18n.locale, while it is not really there.

[Sebastian Noack, 2017-08-28 19:26:56]

LGTM

[Thomas Greiner, 2017-08-29 08:51:17]

Just a small but critical typo and an optional suggestion.

https://codereview.adblockplus.org/29345232/diff/29529763/background.js
File background.js (right):

https://codereview.adblockplus.org/29345232/diff/29529763/background.js#newco...
background.js:98: reuturn bidiDir =
/^(ar|fa|he|ug|ur)(-|$)/.test(this.appLocale) ?
Typo: Replace "reuturn" with "return"

https://codereview.adblockplus.org/29345232/diff/29529763/background.js#newco...
background.js:98: reuturn bidiDir =
/^(ar|fa|he|ug|ur)(-|$)/.test(this.appLocale) ?
Suggestion: We're not using the values in the capture groups here and we may not
care about which character comes after the prefix so my suggestion would be
this: /^(?:ar|fa|he|ug|ur)\b/

[Oleksandr, 2017-08-29 11:26:30]

https://codereview.adblockplus.org/29345232/diff/29529763/background.js
File background.js (right):

https://codereview.adblockplus.org/29345232/diff/29529763/background.js#newco...
background.js:98: reuturn bidiDir =
/^(ar|fa|he|ug|ur)(-|$)/.test(this.appLocale) ?
On 2017/08/29 08:51:17, Thomas Greiner wrote:
> Typo: Replace "reuturn" with "return"

Rebase artifact. There was more than just this too. Thanks!

[Thomas Greiner, 2017-08-29 11:43:07]

LGTM