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

Issue 29346916: Issue 3220 - FAQ and Privacy Policy links redirect to English site only (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 5 months ago by diegocarloslima
Modified:
2 years, 11 months ago
Reviewers:
Felix Dahlke, anton
CC:
René Jeschke
Visibility:
Public.

Description

Issue 3220 - FAQ and Privacy Policy links redirect to English site only

Patch Set 1 #

Total comments: 14

Patch Set 2 : Applying changes from code review #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -10 lines) Patch
M mobile/android/app/mobile.js View 1 1 chunk +5 lines, -3 lines 0 comments Download
M mobile/android/base/resources/xml/preferences_vendor.xml View 1 chunk +1 line, -2 lines 0 comments Download
M mobile/android/base/strings.xml.in View 1 1 chunk +3 lines, -2 lines 0 comments Download
M mobile/android/chrome/content/aboutAdblockBrowser.xhtml View 1 2 chunks +21 lines, -3 lines 3 comments Download

Messages

Total messages: 10
diegocarloslima
3 years, 5 months ago (2016-06-22 00:32:57 UTC) #1
Felix Dahlke
Looks good! Just some minor comments. https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/app/mobile.js File mobile/android/app/mobile.js (right): https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/app/mobile.js#newcode490 mobile/android/app/mobile.js:490: pref("app.faqURL", "https://adblockplus.org/redirect?link=adblock_browser_android_faq&lang=%LOCALE%"); Since ...
3 years, 2 months ago (2016-09-15 14:59:23 UTC) #2
anton
On 2016/06/22 00:32:57, diegocarloslima wrote: LGTM
3 years, 2 months ago (2016-09-30 06:25:41 UTC) #3
Felix Dahlke
Please note that I've posted some comments that weren't addressed yet, so NOT LGTM from ...
3 years, 2 months ago (2016-09-30 06:35:47 UTC) #4
diegocarloslima
https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/app/mobile.js File mobile/android/app/mobile.js (right): https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/app/mobile.js#newcode490 mobile/android/app/mobile.js:490: pref("app.faqURL", "https://adblockplus.org/redirect?link=adblock_browser_android_faq&lang=%LOCALE%"); On 2016/09/15 14:59:22, Felix Dahlke wrote: > ...
3 years, 1 month ago (2016-10-21 13:37:43 UTC) #5
Felix Dahlke
https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/base/preferences/GeckoPreferences.java File mobile/android/base/preferences/GeckoPreferences.java (right): https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/base/preferences/GeckoPreferences.java#newcode920 mobile/android/base/preferences/GeckoPreferences.java:920: final String url = getResources().getString(R.string.faq_link, LOCALE); On 2016/10/21 13:37:43, ...
3 years, 1 month ago (2016-10-28 10:14:52 UTC) #6
diegocarloslima
https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/base/preferences/GeckoPreferences.java File mobile/android/base/preferences/GeckoPreferences.java (right): https://codereview.adblockplus.org/29346916/diff/29346917/mobile/android/base/preferences/GeckoPreferences.java#newcode920 mobile/android/base/preferences/GeckoPreferences.java:920: final String url = getResources().getString(R.string.faq_link, LOCALE); On 2016/10/28 10:14:52, ...
3 years, 1 month ago (2016-11-02 08:55:48 UTC) #7
Felix Dahlke
Almost there :) https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chrome/content/aboutAdblockBrowser.xhtml File mobile/android/chrome/content/aboutAdblockBrowser.xhtml (right): https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chrome/content/aboutAdblockBrowser.xhtml#newcode108 mobile/android/chrome/content/aboutAdblockBrowser.xhtml:108: // Using formatted URLs. See https://issues.adblockplus.org/ticket/3220 ...
3 years ago (2016-11-17 18:34:38 UTC) #8
diegocarloslima
https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chrome/content/aboutAdblockBrowser.xhtml File mobile/android/chrome/content/aboutAdblockBrowser.xhtml (right): https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chrome/content/aboutAdblockBrowser.xhtml#newcode108 mobile/android/chrome/content/aboutAdblockBrowser.xhtml:108: // Using formatted URLs. See https://issues.adblockplus.org/ticket/3220 On 2016/11/17 18:34:37, ...
3 years ago (2016-11-21 14:37:41 UTC) #9
Felix Dahlke
2 years, 11 months ago (2016-12-12 09:57:23 UTC) #10
LGTM

https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chro...
File mobile/android/chrome/content/aboutAdblockBrowser.xhtml (right):

https://codereview.adblockplus.org/29346916/diff/29361530/mobile/android/chro...
mobile/android/chrome/content/aboutAdblockBrowser.xhtml:108: // Using formatted
URLs. See https://issues.adblockplus.org/ticket/3220
On 2016/11/21 14:37:41, diegocarloslima wrote:
> On 2016/11/17 18:34:37, Felix Dahlke wrote:
> > Don't we revert this to the original upstream version here? Because in that
> case
> > we shouldn't add a comment here then.
> 
> Yes, this is a snippet taken from the upstream code 'about.js'. But this is
> slightly different from the upstream version. In the links declaration, we
just
> use faqURL, privacyURL and creditsURL, while they also have releaseNotesUrl
and
> supportURL. Should I place some comment above the line 'let links = [' , then?

Just realised this is not upstream code but rather our copy. So nevermind :)
Sign in to reply to this message.

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