New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1423716: stop adding the SEO root title to all document titles #5142

Merged
merged 1 commit into from Nov 29, 2018

Conversation

Projects
None yet
2 participants
@davidflanagan
Copy link
Collaborator

davidflanagan commented Nov 28, 2018

We currently constuct the <title> of a document like this:

<title> - <SEO root document title> | MDN

So for https://developer.mozilla.org/en-US/docs/Learn/CSS we have:

Learn to style HTML using CSS - Learn web development | MDN

This patch shortens the title by removing the SEO root title, leaving:

Learn to style HTML using CSS | MDN

@davidflanagan davidflanagan requested a review from escattone Nov 28, 2018

@davidflanagan

This comment has been minimized.

Copy link
Collaborator

davidflanagan commented Nov 28, 2018

screen shot 2018-11-27 at 5 15 05 pm

@davidflanagan

This comment has been minimized.

Copy link
Collaborator

davidflanagan commented Nov 28, 2018

So it looks like I need to learn to run the tests locally before I submit a PR...

Bug 1423716: stop adding the SEO root title to all document titles
We currently constuct the <title> of a document like this:

  <title> - <SEO root document title> | MDN

So for https://developer.mozilla.org/en-US/docs/Learn/CSS we have:

  Learn to style HTML using CSS - Learn web development | MDN

This patch shortens the title by removing the SEO root title, leaving:

  Learn to style HTML using CSS | MDN

@davidflanagan davidflanagan force-pushed the davidflanagan:simplify-titles branch from f80d3f5 to ff5f81e Nov 28, 2018

@escattone escattone self-assigned this Nov 29, 2018

@escattone
Copy link
Member

escattone left a comment

This looks and works great, thanks @davidflanagan!

# document in the title of every page, and this test tested for it.
# That feature has been removed now, and now this test verifies that
# the SEO root title does *not* appear in document titles.
def test_no_seo_title(self):

This comment has been minimized.

@escattone

escattone Nov 29, 2018

Member

For me, I think something like test_no_seo_root_in_title would be clearer, but this is fine too.

@escattone escattone merged commit de1c22b into mozilla:master Nov 29, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (mdn) No new, high severity issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment