diff --git a/addons/http_routing/models/ir_http.py b/addons/http_routing/models/ir_http.py index c9d08f3c754..8ed7527232c 100644 --- a/addons/http_routing/models/ir_http.py +++ b/addons/http_routing/models/ir_http.py @@ -418,14 +418,16 @@ class IrHttp(models.AbstractModel): nearest_lang = not func and cls.get_nearest_lang(request.env['res.lang']._lang_get_code(path[1])) url_lang = nearest_lang and path[1] - # if lang in url but not the displayed or default language --> change or remove - # or no lang in url, and lang to dispay not the default language --> add lang - # and not a POST request - # and not a bot or bot but default lang in url - if ((url_lang and (url_lang != request.lang.url_code or url_lang == default_lg_id.url_code)) - or (not url_lang and request.is_frontend_multilang and request.lang != default_lg_id) - and request.httprequest.method != 'POST') \ - and (not is_a_bot or (url_lang and url_lang == default_lg_id.url_code)): + # The default lang should never be in the URL, and a wrong lang + # should never be in the URL. + wrong_url_lang = url_lang and (url_lang != request.lang.url_code or url_lang == default_lg_id.url_code) + # The lang is missing from the URL if multi lang is enabled for + # the route and the current lang is not the default lang. + # POST requests are excluded from this condition. + missing_url_lang = not url_lang and request.is_frontend_multilang and request.lang != default_lg_id and request.httprequest.method != 'POST' + # Bots should never be redirected when the lang is missing + # because it is the only way for them to index the default lang. + if wrong_url_lang or (missing_url_lang and not is_a_bot): if url_lang: path.pop(1) if request.lang != default_lg_id: @@ -440,6 +442,12 @@ class IrHttp(models.AbstractModel): path.pop(1) routing_error = None return cls.reroute('/'.join(path) or '/') + elif missing_url_lang and is_a_bot: + # Ensure that if the URL without lang is not redirected, the + # current lang is indeed the default lang, because it is the + # lang that bots should index in that case. + request.lang = default_lg_id + request.context = dict(request.context, lang=default_lg_id.code) if request.lang == default_lg_id: context = dict(request.context) diff --git a/addons/website/models/website.py b/addons/website/models/website.py index 29398c35872..338cdba591b 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -7,7 +7,9 @@ import logging import hashlib import re + from werkzeug import urls +from werkzeug.datastructures import OrderedMultiDict from werkzeug.exceptions import NotFound from odoo import api, fields, models, tools @@ -437,37 +439,42 @@ class Website(models.Model): # Languages # ---------------------------------------------------------- - def get_alternate_languages(self, req=None): + def _get_alternate_languages(self, canonical_params): + self.ensure_one() + + if not self._is_canonical_url(canonical_params=canonical_params): + # no hreflang on non-canonical pages + return [] + + languages = self.language_ids + if len(languages) <= 1: + # no hreflang if no alternate language + return [] + langs = [] - if req is None: - req = request.httprequest - default = self.get_current_website().default_lang_id.url_code shorts = [] - def get_url_localized(router, lang): - arguments = dict(request.endpoint_arguments) - for key, val in list(arguments.items()): - if isinstance(val, models.BaseModel): - arguments[key] = val.with_context(lang=lang) - return router.build(request.endpoint, arguments) - - router = request.httprequest.app.get_db_router(request.db).bind('') - for lg in self.language_ids: - lg_path = ('/' + lg.url_code) if lg.url_code != default else '' + for lg in languages: lg_codes = lg.code.split('_') - shorts.append(lg_codes[0]) - uri = get_url_localized(router, lg.url_code) if request.endpoint else request.httprequest.path - if req.query_string: - uri += u'?' + req.query_string.decode('utf-8') - lang = { + short = lg_codes[0] + shorts.append(short) + langs.append({ 'hreflang': ('-'.join(lg_codes)).lower(), - 'short': lg_codes[0], - 'href': req.url_root[0:-1] + lg_path + uri, - } - langs.append(lang) + 'short': short, + 'href': self._get_canonical_url_localized(lang=lg, canonical_params=canonical_params), + }) + + # if there is only one region for a language, use only the language code for lang in langs: if shorts.count(lang['short']) == 1: lang['hreflang'] = lang['short'] + + # add the default + langs.append({ + 'hreflang': 'x-default', + 'href': self._get_canonical_url_localized(lang=self.default_lang_id, canonical_params=canonical_params), + }) + return langs # ---------------------------------------------------------- @@ -827,6 +834,55 @@ class Website(models.Model): self.ensure_one() return self._get_http_domain() or super(BaseModel, self).get_base_url() + def _get_canonical_url_localized(self, lang, canonical_params): + """Returns the canonical URL for the current request with translatable + elements appropriately translated in `lang`. + + If `request.endpoint` is not true, returns the current `path` instead. + + `url_quote_plus` is applied on the returned path. + """ + self.ensure_one() + if request.endpoint: + router = request.httprequest.app.get_db_router(request.db).bind('') + arguments = dict(request.endpoint_arguments) + for key, val in list(arguments.items()): + if isinstance(val, models.BaseModel): + if val.env.context.get('lang') != lang.url_code: + arguments[key] = val.with_context(lang=lang.url_code) + path = router.build(request.endpoint, arguments) + else: + # The build method returns a quoted URL so convert in this case for consistency. + path = urls.url_quote_plus(request.httprequest.path, safe='/') + lang_path = ('/' + lang.url_code) if lang != self.default_lang_id else '' + canonical_query_string = '?%s' % urls.url_encode(canonical_params) if canonical_params else '' + return self.get_base_url() + lang_path + path + canonical_query_string + + def _get_canonical_url(self, canonical_params): + """Returns the canonical URL for the current request.""" + self.ensure_one() + return self._get_canonical_url_localized(lang=request.lang, canonical_params=canonical_params) + + def _is_canonical_url(self, canonical_params): + """Returns whether the current request URL is canonical.""" + self.ensure_one() + # Compare OrderedMultiDict because the order is important, there must be + # only one canonical and not params permutations. + params = request.httprequest.args + canonical_params = canonical_params or OrderedMultiDict() + if params != canonical_params: + return False + # Compare URL at the first rerouting iteration (if available) because + # it's the one with the language in the path. + # It is important to also test the domain of the current URL. + current_url = request.httprequest.url_root[:-1] + (hasattr(request, 'rerouting') and request.rerouting[0] or request.httprequest.path) + canonical_url = self._get_canonical_url_localized(lang=request.lang, canonical_params=None) + # A request path with quotable characters (such as ",") is never + # canonical because request.httprequest.base_url is always unquoted, + # and canonical url is always quoted, so it is never possible to tell + # if the current URL is indeed canonical or not. + return current_url == canonical_url + class BaseModel(models.AbstractModel): _inherit = 'base' diff --git a/addons/website/tests/test_base_url.py b/addons/website/tests/test_base_url.py index 421174d0c65..18d296f38e9 100644 --- a/addons/website/tests/test_base_url.py +++ b/addons/website/tests/test_base_url.py @@ -1,22 +1,40 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import odoo +from lxml.html import document_fromstring + import odoo.tests -@odoo.tests.tagged('-at_install', 'post_install') -class TestBaseUrl(odoo.tests.HttpCase): - def test_base_url(self): - ICP = self.env['ir.config_parameter'] - Website = self.env['website'] +class TestUrlCommon(odoo.tests.HttpCase): + def setUp(self): + super(TestUrlCommon, self).setUp() + self.domain = 'http://' + odoo.tests.HOST + self.website = self.env['website'].create({ + 'name': 'test base url', + 'domain': self.domain, + }) + lang_fr = self.env.ref('base.lang_fr') + lang_fr.write({'active': True}) + self.website.language_ids = self.env.ref('base.lang_en') + lang_fr + self.website.default_lang_id = self.env.ref('base.lang_en') + + def _assertCanonical(self, url, canonical_url): + res = self.url_open(url) + canonical_link = document_fromstring(res.content).xpath("/html/head/link[@rel='canonical']") + self.assertEqual(len(canonical_link), 1) + self.assertEqual(canonical_link[0].attrib["href"], canonical_url) + + +@odoo.tests.tagged('-at_install', 'post_install') +class TestBaseUrl(TestUrlCommon): + def test_01_base_url(self): + ICP = self.env['ir.config_parameter'] icp_base_url = ICP.sudo().get_param('web.base.url') - domain = 'https://www.domain.jke' - website = Website.create({'name': 'test base url', 'domain': domain}) # Test URL is correct for the website itself when the domain is set - self.assertEqual(website.get_base_url(), domain) + self.assertEqual(self.website.get_base_url(), self.domain) # Test URL is correct for a model without website_id without_website_id = self.env['ir.attachment'].create({'name': 'test base url'}) @@ -30,12 +48,19 @@ class TestBaseUrl(odoo.tests.HttpCase): self.assertEqual(with_website_id.get_base_url(), icp_base_url) # ...when the website is correctly set - with_website_id.website_id = website - self.assertEqual(with_website_id.get_base_url(), domain) + with_website_id.website_id = self.website + self.assertEqual(with_website_id.get_base_url(), self.domain) # ...when the set website doesn't have a domain - website.domain = False + self.website.domain = False self.assertEqual(with_website_id.get_base_url(), icp_base_url) # Test URL is correct for the website itself when no domain is set - self.assertEqual(website.get_base_url(), icp_base_url) + self.assertEqual(self.website.get_base_url(), icp_base_url) + + def test_02_canonical_url(self): + self._assertCanonical('/', self.domain + '/') + self._assertCanonical('/?debug=1', self.domain + '/') + self._assertCanonical('/a-page', self.domain + '/a-page') + self._assertCanonical('/en_US', self.domain + '/') + self._assertCanonical('/fr_FR', self.domain + '/fr/') diff --git a/addons/website/views/website_templates.xml b/addons/website/views/website_templates.xml index 373b8d524db..83c68dfb959 100644 --- a/addons/website/views/website_templates.xml +++ b/addons/website/views/website_templates.xml @@ -228,10 +228,12 @@ - + + + diff --git a/addons/website_event/controllers/main.py b/addons/website_event/controllers/main.py index edd01ebbe19..6ee8b01a42b 100644 --- a/addons/website_event/controllers/main.py +++ b/addons/website_event/controllers/main.py @@ -3,7 +3,7 @@ import babel.dates import re import werkzeug -import json +from werkzeug.datastructures import OrderedMultiDict from datetime import datetime, timedelta from dateutil.relativedelta import relativedelta @@ -28,6 +28,7 @@ class WebsiteEventController(http.Controller): searches.setdefault('type', 'all') searches.setdefault('country', 'all') + website = request.website def sdn(date): return fields.Datetime.to_string(date.replace(hour=23, minute=59, second=59)) @@ -63,7 +64,7 @@ class WebsiteEventController(http.Controller): ] # search domains - domain_search = {'website_specific': request.website.website_domain()} + domain_search = {'website_specific': website.website_domain()} current_date = None current_type = None current_country = None @@ -110,7 +111,7 @@ class WebsiteEventController(http.Controller): step = 10 # Number of events per page event_count = Event.search_count(dom_without("none")) - pager = request.website.pager( + pager = website.pager( url="/event", url_args={'date': searches.get('date'), 'type': searches.get('type'), 'country': searches.get('country')}, total=event_count, @@ -139,6 +140,10 @@ class WebsiteEventController(http.Controller): 'search_path': "?%s" % werkzeug.url_encode(searches), } + if searches['date'] == 'old': + # the only way to display this content is to set date=old so it must be canonical + values['canonical_params'] = OrderedMultiDict([('date', 'old')]) + return request.render("website_event.index", values) @http.route(['''/event//page/'''], type='http', auth="public", website=True, sitemap=False) diff --git a/addons/website_event/tests/test_event_website.py b/addons/website_event/tests/test_event_website.py index bc8e9b18a1e..b01c975bb71 100644 --- a/addons/website_event/tests/test_event_website.py +++ b/addons/website_event/tests/test_event_website.py @@ -2,6 +2,8 @@ from datetime import datetime, timedelta from odoo import fields from odoo.addons.event.tests.common import TestEventCommon +from odoo.addons.website.tests.test_base_url import TestUrlCommon +import odoo.tests class TestEventWebsiteHelper(TestEventCommon): @@ -35,3 +37,10 @@ class TestEventWebsite(TestEventWebsiteHelper): self.assertFalse(self.event_0.menu_id) self.event_0.website_menu = True self._assert_website_menus(self.event_0) + + +@odoo.tests.tagged('-at_install', 'post_install') +class TestUrlCanonical(TestUrlCommon): + def test_01_canonical_url(self): + self._assertCanonical('/event?date=all', self.domain + '/event') + self._assertCanonical('/event?date=old', self.domain + '/event?date=old') diff --git a/addons/website_slides/views/website_slides_templates_homepage.xml b/addons/website_slides/views/website_slides_templates_homepage.xml index af2f8bf9346..33f84388453 100644 --- a/addons/website_slides/views/website_slides_templates_homepage.xml +++ b/addons/website_slides/views/website_slides_templates_homepage.xml @@ -238,7 +238,7 @@ t-esc="tag_group.name"/>
-