includes/sidebar/primary-navigation.tmpl - Issue 29490624: Issue 5373 - Remove the "blog" from acceptableads.com and link to blog.acceptableads.com

Side by Side Diff: includes/sidebar/primary-navigation.tmpl

Issue 29490624: Issue 5373 - Remove the "blog" from acceptableads.com and link to blog.acceptableads.com (Closed) Base URL: https://hg.adblockplus.org/web.acceptableads.com

Patch Set: Created July 17, 2017, 12:42 p.m.

Left:
Right:

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

Jump to:

View unified diff | Download patch

OLD	NEW
1 {#	1 {#

2 # This file is part of acceptableads.org.	2 # This file is part of acceptableads.org.

3 # Copyright (C) 2016 Eyeo GmbH	3 # Copyright (C) 2016 Eyeo GmbH

4 #	4 #

5 # acceptableads.org is free software: you can redistribute it and/or modify	5 # acceptableads.org is free software: you can redistribute it and/or modify

6 # it under the terms of the GNU General Public License as published by	6 # it under the terms of the GNU General Public License as published by

7 # the Free Software Foundation, either version 3 of the License, or	7 # the Free Software Foundation, either version 3 of the License, or

8 # (at your option) any later version.	8 # (at your option) any later version.

9 #	9 #

10 # acceptableads.org is distributed in the hope that it will be useful,	10 # acceptableads.org is distributed in the hope that it will be useful,

11 # but WITHOUT ANY WARRANTY; without even the implied warranty of	11 # but WITHOUT ANY WARRANTY; without even the implied warranty of

12 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the	12 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

13 # GNU General Public License for more details.	13 # GNU General Public License for more details.

14 #	14 #

15 # You should have received a copy of the GNU General Public License	15 # You should have received a copy of the GNU General Public License

16 # along with acceptableads.org. If not, see <http://www.gnu.org/licenses/>.	16 # along with acceptableads.org. If not, see <http://www.gnu.org/licenses/>.

17 #}	17 #}

18	18

19 <ul>	19 <ul>

20 {% for sitemap_page, sitemap_parent, sitemap_children in sitemap %}	20 {% for sitemap_page, sitemap_parent, sitemap_children in sitemap %}

21 <li class="{% if parent == sitemap_page or page == sitemap_page %} active {% e ndif %} {% if sitemap_children \| length > 0 %} has-submenu {% endif %}">	21 {% if 'http' in sitemap_page %}

22 {{ sitemap_page \| linkify }}{{ sitemap_parent \| translate(sitemap_page, "Pri mary navigation label") }}</a>	22 <li>

23 {% if sitemap_children \| length > 0 %}	23 <a href="{{ sitemap_page }}" target="_blank" rel="noopener noreferrer">
	ire 2017/07/17 12:46:20 Not sure if we typically add this, but I read that Not sure if we typically add this, but I read that it should be included for security/performance reasons: - https://jakearchibald.com/2016/performance-benefits-of-rel-noopener/ - https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vul... juliandoucette 2017/07/18 16:58:50 Does this apply to any href? - We may want the ref Show quoted text On 2017/07/17 12:46:20, ire wrote: > Not sure if we typically add this, but I read that it should be included for > security/performance reasons: > > - https://jakearchibald.com/2016/performance-benefits-of-rel-noopener/ > - > https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vul... Does this apply to any href? - We may want the referrer for stats on traffic from aa.com to the blog - I think we can trust medium.com not to attack us? ire 2017/07/19 12:55:22 From what I can tell it seems like a good idea to, Show quoted text On 2017/07/18 16:58:50, juliandoucette wrote: > On 2017/07/17 12:46:20, ire wrote: > > Not sure if we typically add this, but I read that it should be included for > > security/performance reasons: > > > > - https://jakearchibald.com/2016/performance-benefits-of-rel-noopener/ > > - > > > https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vul... > > Does this apply to any href? > - We may want the referrer for stats on traffic from http://aa.com to the blog > - I think we can trust http://medium.com not to attack us? From what I can tell it seems like a good idea to, as a default, apply this to external links. In this case, perhaps the noreferrer value should be excluded since we are linking to our own blog and may want the stats. Regarding the nofollow value, you're probably right that we can trust medium.com not to attack us. However, there may still be some performance benefits to be gained from using it. What do you think? Is it worth including at all? juliandoucette 2017/07/19 15:03:31 - I want to exclude `noreferrer` in this case - I Show quoted text On 2017/07/19 12:55:22, ire wrote: > From what I can tell it seems like a good idea to, as a default, apply this to > external links. In this case, perhaps the noreferrer value should be excluded > since we are linking to our own blog and may want the stats. Regarding the > nofollow value, you're probably right that we can trust http://medium.com not to attack > us. However, there may still be some performance benefits to be gained from > using it. What do you think? Is it worth including at all? - I want to exclude `noreferrer` in this case - I want to keep `noopener` in general By this logic, we should add both of these `rel`s to `a`s by default and exclude `noreferrer` if the `href` is whitelisted. But I think it's a better idea to do this automatically via the CMS (in a separate ticket) than here (and elsewhere in content). I vote we remove the `rel`s here (which pose no real threat or significant performance benefit) and create a separate Sitescripts issue to address the potential threats and benefits globally elsewhere. ire 2017/07/19 18:10:10 I agree. It definitely makes sense to have this au Show quoted text On 2017/07/19 15:03:31, juliandoucette wrote: > On 2017/07/19 12:55:22, ire wrote: > > From what I can tell it seems like a good idea to, as a default, apply this to > > external links. In this case, perhaps the noreferrer value should be excluded > > since we are linking to our own blog and may want the stats. Regarding the > > nofollow value, you're probably right that we can trust http://medium.com not > to attack > > us. However, there may still be some performance benefits to be gained from > > using it. What do you think? Is it worth including at all? > > - I want to exclude `noreferrer` in this case > - I want to keep `noopener` in general > > By this logic, we should add both of these `rel`s to `a`s by default and exclude > `noreferrer` if the `href` is whitelisted. But I think it's a better idea to do > this automatically via the CMS (in a separate ticket) than here (and elsewhere > in content). > > I vote we remove the `rel`s here (which pose no real threat or significant > performance benefit) and create a separate Sitescripts issue to address the > potential threats and benefits globally elsewhere. I agree. It definitely makes sense to have this automated. I will remove the attributes from this case. ire 2017/07/19 18:10:10 Done. Show quoted text On 2017/07/19 15:03:31, juliandoucette wrote: > On 2017/07/19 12:55:22, ire wrote: > > From what I can tell it seems like a good idea to, as a default, apply this to > > external links. In this case, perhaps the noreferrer value should be excluded > > since we are linking to our own blog and may want the stats. Regarding the > > nofollow value, you're probably right that we can trust http://medium.com not > to attack > > us. However, there may still be some performance benefits to be gained from > > using it. What do you think? Is it worth including at all? > > - I want to exclude `noreferrer` in this case > - I want to keep `noopener` in general > > By this logic, we should add both of these `rel`s to `a`s by default and exclude > `noreferrer` if the `href` is whitelisted. But I think it's a better idea to do > this automatically via the CMS (in a separate ticket) than here (and elsewhere > in content). > > I vote we remove the `rel`s here (which pose no real threat or significant > performance benefit) and create a separate Sitescripts issue to address the > potential threats and benefits globally elsewhere. Done.
24 <ul>	24 {{ sitemap_parent \| translate(sitemap_parent, "Primary navigation label" ) }}

25 {% for sitemap_page, sitemap_child in sitemap_children %}	25 <img src="/img/png/icon-external-link.png" srcset="/img/png/icon-externa l-link.png, /img/svg/icon-external-link.svg 2x" alt="External Link" class="exter nal-icon">
	juliandoucette 2017/07/18 16:58:50 - missing translate filter on alt text - the icon - missing translate filter on alt text - the icon should be white - the 1x icon should be 1x size ire 2017/07/19 12:55:22 Done. Show quoted text On 2017/07/18 16:58:50, juliandoucette wrote: > - missing translate filter on alt text > - the icon should be white > - the 1x icon should be 1x size Done.
26 <li class="{% if page == sitemap_page %} active {% endif %}">	26 </a>

27 {{ sitemap_page \| linkify }}{{ sitemap_child \| translate(sitemap_page, "Primary navigation label under \"" + sitemap_parent + "\"") }}</a>	27 </li>

28 </li>	28 {% else %}

29 {% endfor %}	29 <li class="{% if parent == sitemap_page or page == sitemap_page %} active {% endif %} {% if sitemap_children \| length > 0 %} has-submenu {% endif %}">

30 </ul>	30 {{ sitemap_page \| linkify }}{{ sitemap_parent \| translate(sitemap_page, "P rimary navigation label") }}</a>

31 {% endif %}	31 {% if sitemap_children \| length > 0 %}

32 </li>	32 <ul>

	33 {% for sitemap_page, sitemap_child in sitemap_children %}

	34 <li class="{% if page == sitemap_page %} active {% endif %}">

	35 {{ sitemap_page \| linkify }}{{ sitemap_child \| translate(sitemap_pag e, "Primary navigation label under \"" + sitemap_parent + "\"") }}</a>

	36 </li>

	37 {% endfor %}

	38 </ul>

	39 {% endif %}

	40 </li>

	41 {% endif %}

33 {% endfor %}	42 {% endfor %}

34 </ul>	43 </ul>

OLD	NEW

« README.md ('K') | « includes/breadcrumbs.tmpl ('k') | pages/blog/first-update.md » ('j') | no next file with comments »