Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(375)

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
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed comments #

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
Wladimir Palant
4 years, 1 month ago (2015-10-29 13:25:58 UTC) #1
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5