mobile/android/chrome/content/aboutAdblockBrowser.xhtml - Issue 29346916: Issue 3220 - FAQ and Privacy Policy links redirect to English site only

Delta Between Two Patch Sets: mobile/android/chrome/content/aboutAdblockBrowser.xhtml

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

Left Patch Set: Created June 22, 2016, 12:31 a.m.

Right Patch Set: Applying changes from code review Created Nov. 2, 2016, 10:08 a.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 <?xml version="1.0" encoding="UTF-8"?>	1 <?xml version="1.0" encoding="UTF-8"?>

2	2

3 <!DOCTYPE html [	3 <!DOCTYPE html [

4 <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" >	4 <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" >

5 %brandDTD;	5 %brandDTD;

6 <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">	6 <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">

7 %globalDTD;	7 %globalDTD;

8 <!ENTITY % fennecDTD SYSTEM "chrome://browser/locale/about.dtd">	8 <!ENTITY % fennecDTD SYSTEM "chrome://browser/locale/about.dtd">

9 %fennecDTD;	9 %fennecDTD;

10 ]>	10 ]>

(...skipping 87 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
98	98

99 let distroAbout = Services.prefs.getComplexValue("distribution.about", Ci.nsISupportsString);	99 let distroAbout = Services.prefs.getComplexValue("distribution.about", Ci.nsISupportsString);

100 let distroField = document.getElementById("distributionAbout");	100 let distroField = document.getElementById("distributionAbout");

101 distroField.textContent = distroAbout;	101 distroField.textContent = distroAbout;

102 distroField.hidden = false;	102 distroField.hidden = false;

103 }	103 }

104 } catch (e) {	104 } catch (e) {

105 // Pref is unset	105 // Pref is unset

106 }	106 }

107	107

108 // Using formatted URLs. See https://issues.adblockplus.org/ticket/3220	108 // Using formatted URLs. See https://issues.adblockplus.org/ticket/3220
	Felix Dahlke 2016/11/17 18:34:37 Don't we revert this to the original upstream vers Don't we revert this to the original upstream version here? Because in that case we shouldn't add a comment here then. diegocarloslima 2016/11/21 14:37:41 Yes, this is a snippet taken from the upstream cod Show quoted text 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? Felix Dahlke 2016/12/12 09:57:22 Just realised this is not upstream code but rather Show quoted text 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 :)
109 // get URLs from prefs	109 // get URLs from prefs

110 try {	110 try {
Felix Dahlke 2016/09/15 14:59:22 Something seems wrong here with the indentation. Something seems wrong here with the indentation. diegocarloslima 2016/10/21 13:37:43 Acknowledged. Show quoted text On 2016/09/15 14:59:22, Felix Dahlke wrote: > Something seems wrong here with the indentation. Acknowledged.
111 let formatter = Cc["@mozilla.org/toolkit/URLFormatterService;1"].getServic e(Ci.nsIURLFormatter);	111 let formatter = Cc["@mozilla.org/toolkit/URLFormatterService;1"].getServ ice(Ci.nsIURLFormatter);

112	112

113 let links = [	113 let links = [

114 {id: "faqURL", pref: "app.faqURL"},	114 {id: "faqURL", pref: "app.faqURL"},
Felix Dahlke 2016/09/15 14:59:22 Given that we copied this code from about.js, we c Given that we copied this code from about.js, we can adjust it to our coding style. So just one space after , - we usually avoid column indentation. diegocarloslima 2016/10/21 13:37:43 Acknowledged. Show quoted text On 2016/09/15 14:59:22, Felix Dahlke wrote: > Given that we copied this code from about.js, we can adjust it to our coding > style. So just one space after , - we usually avoid column indentation. Acknowledged.
115 {id: "privacyURL", pref: "app.privacyURL"},	115 {id: "privacyURL", pref: "app.privacyURL"},

116 {id: "creditsURL", pref: "app.creditsURL"},	116 {id: "creditsURL", pref: "app.creditsURL"},

117 ];	117 ];

118	118

119 links.forEach(function(link) {	119 links.forEach(function(link) {

120 let url = formatter.formatURLPref(link.pref);	120 let url = formatter.formatURLPref(link.pref);

121 let element = document.getElementById(link.id);	121 let element = document.getElementById(link.id);

122 element.setAttribute("href", url);	122 element.setAttribute("href", url);

123 });	123 });

124 } catch (ex) {}	124 } catch (ex) {}

125	125

126 #ifdef MOZ_UPDATER	126 #ifdef MOZ_UPDATER

127 let Updater = {	127 let Updater = {

128 update: null,	128 update: null,

129	129

130 init: function() {	130 init: function() {

131 Services.obs.addObserver(this, "Update:CheckResult", false);	131 Services.obs.addObserver(this, "Update:CheckResult", false);

132 },	132 },

133	133

134 observe: function(aSubject, aTopic, aData) {	134 observe: function(aSubject, aTopic, aData) {

(...skipping 68 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
203 break;	203 break;

204 case "DOWNLOADED":	204 case "DOWNLOADED":

205 downloadedSpan.style.display = "block";	205 downloadedSpan.style.display = "block";

206 break;	206 break;

207 }	207 }

208 }	208 }

209 #endif	209 #endif

210 ]]></script>	210 ]]></script>

211 </body>	211 </body>

212 </html>	212 </html>

LEFT	RIGHT