|
|
Created:
July 5, 2017, 5:34 p.m. by juliandoucette Modified:
July 18, 2017, 9:57 a.m. Base URL:
https://hg.adblockplus.org/website-defaults Visibility:
Public. |
DescriptionIssue 5329 - Added standard meta data to website-defaults
Patch Set 1 #
Total comments: 6
Patch Set 2 : Removed title condition #MessagesTotal messages: 8
I decided to break up this issue into several reviews for the sake of efficiency. I'll separate: - license header - standard meta data - standard social media meta data - article specific meta data - ...
Thanks, this is much better broken up. https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} Question: Since these are the standard meta data that *must* be on every page, is there a way to enforce them? i.e. will things break if there is no title or description? If not, should we have some form of fallback? e.g. use the site-wide title and description if a page-specific one is not provided. This may not actually matter if the code will never pass review without having a title and description for each page.
https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} On 2017/07/06 08:12:02, ire wrote: > Question: Since these are the standard meta data that *must* be on every page, > is there a way to enforce them? i.e. will things break if there is no title or > description? If not, should we have some form of fallback? e.g. use the > site-wide title and description if a page-specific one is not provided. > > This may not actually matter if the code will never pass review without having a > title and description for each page. Good question. - I think title and description should be required - I wanted to integrate this without breaking existing websites - I don't want to block this change with the creation of a description for every page that is currently missing one Weak conclusion: They must be conditional.
LGTM See my suggestion below. https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} On 2017/07/06 13:26:00, juliandoucette wrote: > On 2017/07/06 08:12:02, ire wrote: > > Question: Since these are the standard meta data that *must* be on every page, > > is there a way to enforce them? i.e. will things break if there is no title or > > description? If not, should we have some form of fallback? e.g. use the > > site-wide title and description if a page-specific one is not provided. > > > > This may not actually matter if the code will never pass review without having > a > > title and description for each page. > > Good question. > > - I think title and description should be required > - I wanted to integrate this without breaking existing websites > - I don't want to block this change with the creation of a description for every > page that is currently missing one > > Weak conclusion: They must be conditional. Okay I see, makes sense. This is probably a suggestion that can be dealt with in a separate issue, but do you think it would make sense to have a site-wide fallback then? For example, let's say there is no title/description for a specific page on adblockplus.org, the fallback title/description would be something applicable to the entire site, e.g. "Adblock Plus" for the title and "Surf the web without annoying ads!" for the description. Since we have a relatively small number of sites, we could add this without too much effort and guarantee that all pages of every site will have a title and description.
https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} On 2017/07/07 08:37:43, ire wrote: > This is probably a suggestion that can be dealt with in a separate issue, but do > you think it would make sense to have a site-wide fallback then? I don't know. In (my) theory - a competent parser (search engine, social media) will fall back to the highest level heading (title) and the first phrasing content it can find (description) and this may be better than the site title/description because we don't want people going to specific (non-home) pages that look like home in search results / social media. - But I'm really speculating here - and I'm guessing I'm wrong about something. Do you know anything about this?
Sorry for the delay. Answer below. https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} On 2017/07/07 11:50:16, juliandoucette wrote: > On 2017/07/07 08:37:43, ire wrote: > > This is probably a suggestion that can be dealt with in a separate issue, but > do > > you think it would make sense to have a site-wide fallback then? > > I don't know. > > In (my) theory - a competent parser (search engine, social media) will fall back > to the highest level heading (title) and the first phrasing content it can find > (description) and this may be better than the site title/description because we > don't want people going to specific (non-home) pages that look like home in > search results / social media. - But I'm really speculating here - and I'm > guessing I'm wrong about something. > > Do you know anything about this? From my (limited) research, I couldn't really find anything to support this fallback behaviour. Additionally, according to the HTML spec, a title element is required in most circumstances (http://w3c.github.io/html/single-page.html#the-head-element) and a page will fail validation if it is not included. I don't think this is the same for description though. So perhaps a fallback would be necessary for the page title, but not the description.
https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29480630/diff/29480631/includes/meta/stand... includes/meta/standard.tmpl:4: {% if title %} On 2017/07/12 08:22:37, ire wrote: > On 2017/07/07 11:50:16, juliandoucette wrote: > > On 2017/07/07 08:37:43, ire wrote: > > > This is probably a suggestion that can be dealt with in a separate issue, > but > > do > > > you think it would make sense to have a site-wide fallback then? > > > > I don't know. > > > > In (my) theory - a competent parser (search engine, social media) will fall > back > > to the highest level heading (title) and the first phrasing content it can > find > > (description) and this may be better than the site title/description because > we > > don't want people going to specific (non-home) pages that look like home in > > search results / social media. - But I'm really speculating here - and I'm > > guessing I'm wrong about something. > > > > Do you know anything about this? > > From my (limited) research, I couldn't really find anything to support this > fallback behaviour. Additionally, according to the HTML spec, a title element is > required in most circumstances > (http://w3c.github.io/html/single-page.html#the-head-element) and a page will > fail validation if it is not included. > > I don't think this is the same for description though. So perhaps a fallback > would be necessary for the page title, but not the description. Acknowledged. I was wrong about Facebook; at least. See https://developers.facebook.com/tools/debug/sharing/?q=http%3A%2F%2Fjuliandou... |