Issue 29329473: Issue 3222 - Don`t do localization in the contentPolicy module

Issue 29329473: Issue 3222 - Don`t do localization in the contentPolicy module (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
4 years, 1 month ago by Wladimir Palant

Modified:
4 years ago

Reviewers:
tschuster, Thomas Greiner

Visibility:
Public.

More Reviews

Description

Issue 3222 - Don`t do localization in the contentPolicy module

Patch Set 2 : Addressed comments #

Created: 4 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+34 lines, -46 lines)			Patch
M	chrome/content/ui/composer.js	View	1	2 chunks	+2 lines, -4 lines	0 comments	Download
M	chrome/content/ui/sidebar.js	View	1	7 chunks	+14 lines, -6 lines	0 comments	Download
M	lib/contentPolicy.js	View	1	5 chunks	+18 lines, -28 lines	0 comments	Download
M	lib/requestNotifier.js	View	1	1 chunk	+0 lines, -8 lines	0 comments	Download

Messages

Total messages: 4

Expand All Messages | Collapse All Messages

tschuster

https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/composer.js File chrome/content/ui/composer.js (right): https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/composer.js#newcode124 chrome/content/ui/composer.js:124: types.sort(); Seems like sorting [type, label] could be different ...

4 years, 1 month ago (2015-11-05 15:06:15 UTC) #2

Wladimir Palant

Interdiff shows more changes than the two below - that's due to rebasing. https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/composer.js File ...

4 years, 1 month ago (2015-11-05 15:47:11 UTC) #3

tschuster

4 years, 1 month ago (2015-11-06 19:58:20 UTC) #4

On 2015/11/05 15:47:11, Wladimir Palant wrote:
> Interdiff shows more changes than the two below - that's due to rebasing.
> 
>
https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/c...
> File chrome/content/ui/composer.js (right):
> 
>
https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/c...
> chrome/content/ui/composer.js:124: types.sort();
> On 2015/11/05 15:06:15, tschuster wrote:
> > Seems like sorting [type, label] could be different from just type.
> 
> It actually is the same, due to the way arrays are stringified. Then again, I
> think that this isn't a good way to solve this so I reworked this part.
> 
Nice solution :)
>
https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/s...
> File chrome/content/ui/sidebar.js (right):
> 
>
https://codereview.adblockplus.org/29329473/diff/29329474/chrome/content/ui/s...
> chrome/content/ui/sidebar.js:37: var types = new Map();
> On 2015/11/05 15:06:15, tschuster wrote:
> > localizedTypes maybe?
> 
> Done.

LGTM

Expand All Messages | Collapse All Messages