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

Issue 29511696: Issue 5447 - Add missing essential meta data to adblockplus.org (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 4 days ago by juliandoucette
Modified:
5 days, 1 hour ago
CC:
ire, Jon Sonesen
Base URL:
https://hg.adblockplus.org/web.adblockplus.org
Visibility:
Public.

Description

Issue 5447 - Add missing essential meta data to adblockplus.org

Patch Set 1 #

Total comments: 11

Patch Set 2 : Added ignore_browsers filter #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -21 lines) Patch
A filters/ignore_browsers.py View 1 1 chunk +17 lines, -0 lines 2 comments Download
A filters/to_og_locale.py View 1 chunk +33 lines, -0 lines 0 comments Download
M includes/index.tmpl View 1 chunk +1 line, -4 lines 0 comments Download
M pages/acceptable-ads.md View 1 chunk +0 lines, -2 lines 0 comments Download
M pages/bugs.html View 1 chunk +1 line, -2 lines 0 comments Download
M pages/contribute.html View 1 chunk +0 lines, -2 lines 0 comments Download
M pages/deployments.tmpl View 1 chunk +0 lines, -2 lines 0 comments Download
M pages/features.html View 1 chunk +0 lines, -2 lines 0 comments Download
M pages/getting_started.html View 1 chunk +1 line, -2 lines 0 comments Download
M pages/tutorials.html View 1 chunk +1 line, -2 lines 0 comments Download
M settings.ini View 1 chunk +3 lines, -1 line 0 comments Download
M templates/default.tmpl View 1 1 chunk +12 lines, -2 lines 0 comments Download

Messages

Total messages: 11
juliandoucette
1 week, 4 days ago (2017-08-10 21:21:16 UTC) #1
juliandoucette
Can you help out with this Manvel and/or Thomas? https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl#oldcode31 includes/index.tmpl:31: ...
1 week, 4 days ago (2017-08-10 21:32:43 UTC) #2
Thomas Greiner
https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl#oldcode31 includes/index.tmpl:31: <meta content="https://adblockplus.org/" property="og:url"> On 2017/08/10 21:32:42, juliandoucette wrote: > ...
1 week, 4 days ago (2017-08-11 13:15:42 UTC) #3
juliandoucette
Thanks Thomas! Answers below. https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl#oldcode31 includes/index.tmpl:31: <meta content="https://adblockplus.org/" property="og:url"> On 2017/08/11 ...
1 week, 3 days ago (2017-08-11 20:26:59 UTC) #4
Thomas Greiner
https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl#oldcode31 includes/index.tmpl:31: <meta content="https://adblockplus.org/" property="og:url"> On 2017/08/11 20:26:58, juliandoucette wrote: > ...
1 week, 1 day ago (2017-08-14 11:20:53 UTC) #5
juliandoucette
https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl#oldcode31 includes/index.tmpl:31: <meta content="https://adblockplus.org/" property="og:url"> On 2017/08/14 11:20:53, Thomas Greiner wrote: ...
1 week ago (2017-08-15 17:14:26 UTC) #6
Thomas Greiner
Looks fine now. Just one suggestion regarding a more generic approach. https://codereview.adblockplus.org/29511696/diff/29511697/includes/index.tmpl File includes/index.tmpl (left): ...
5 days, 6 hours ago (2017-08-17 12:27:36 UTC) #7
juliandoucette
+CC Vasily, Jon We ran into an issue with get_canonical_url below because adblockplus.org has several ...
5 days, 4 hours ago (2017-08-17 13:50:07 UTC) #8
Thomas Greiner
On 2017/08/17 13:50:07, juliandoucette wrote: > Also @Thomas I think it would be beneficial to ...
5 days, 4 hours ago (2017-08-17 14:00:29 UTC) #9
Vasily Kuznetsov
On 2017/08/17 13:50:07, juliandoucette wrote: > +CC Vasily, Jon > > We ran into an ...
5 days, 4 hours ago (2017-08-17 14:25:13 UTC) #10
juliandoucette
5 days, 1 hour ago (2017-08-17 17:04:18 UTC) #11
On 2017/08/17 14:25:13, Vasily Kuznetsov wrote:
> It seems useful to support page-specific canonical URL overrides but I'm not
> sure yet what method I prefer. Seems like encoding such things into
> `get_canonical_url` is not quite right, and overriding `page` variable can
have
> side-effects, because it's not only used for the canonical URL. Let's create a
> ticket and then think about the options there.

Done.

- https://issues.adblockplus.org/ticket/5530
- https://issues.adblockplus.org/ticket/5531
Sign in to reply to this message.

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