WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30678 closed defect (bug) (fixed)

Twenty Fifteen: Problem with fonts turned off

Reported by: pavelevap Owned by: lancewillett
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

I tried to turn off all Twenty Fifteen fonts in localization files and following URL is called:

<link rel="stylesheet" id="twentyfifteen-fonts-css" href="//fonts.googleapis.com/css?family&amp;subset=latin%2Clatin-ext" type="text/css" media="all">

There is also GET error in browser console. We should not call fonts when they are turned off by translators?

Attachments (2)

30678.patch (730 bytes) - added by SergeyBiryukov 6 years ago.
30678.diff (1.7 KB) - added by obenland 6 years ago.
Set variable at beginning of function and remove dependency on fonts.

Download all attachments as: .zip

Change History (17)

#1 @SergeyBiryukov
6 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.1

Good catch.

#2 @pavelevap
6 years ago

And related problem: I tried to make a child theme for 2015, but when I dequeued twentyfifteen-fonts, then parent style.css was not loaded in child theme because there is following dependency:

wp_enqueue_style( 'twentyfifteen-style', get_stylesheet_uri(), array( 'twentyfifteen-fonts', 'genericons' ) );

I am not sure if it is a bug (or I am doing it wrong), but it should be possible to remove fonts from child theme?

#3 @pavelevap
6 years ago

Child theme was broken here: [30692]

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#4 @SergeyBiryukov
6 years ago

Yes, the 'twentyfifteen-fonts' dependency should probably be removed.

In Twenty Fourteen, we didn't have 'twentyfourteen-lato' as a dependency for the main stylesheet.

@obenland
6 years ago

Set variable at beginning of function and remove dependency on fonts.

#5 @obenland
6 years ago

We should probably remove all dependencies for style.css. Twenty Twelve and Thirteen had no dependencies.

Twenty Fourteen has a Genericon dependency, which doesn't seem good to have. If Genericons are unavailable, the theme should still be styled.

#6 @lancewillett
6 years ago

Removing Genericons dependencies sounds like a new ticket -- keeping this one focused on the fonts issue.

#7 @lancewillett
6 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 30832:

Twenty Fifteen: remove fonts as a style dependency.

Props obenland, SergeyBiryukov. Fixes #30678.

#8 follow-up: @pavelevap
6 years ago

But problem with Genericons is the same. When removed in child theme, no styles will be applied and it is wrong...

#9 @pavelevap
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It should be also committed into 4.1 branch?

#10 in reply to: ↑ 8 @iamtakashi
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to pavelevap:

But problem with Genericons is the same. When removed in child theme, no styles will be applied and it is wrong...

"Removing Genericons dependencies sounds like a new ticket -- keeping this one focused on the fonts issue."

Look Lance's comment above. Feel free to open a new ticket.

#11 @pavelevap
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

For Genericons I will reopen #30532, there is no need for another ticket.

But this ticket was reopened because fix for fonts was committed only to trunk and not to 4.1 branch, even if it is marked for 4.1 milestone.

#12 @nacin
6 years ago

  • Keywords commit fixed-major added

This ticket was mentioned in Slack in #core by lancewillett. View the logs.


6 years ago

#14 @johnbillion
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 30881:

Twenty Fifteen: remove fonts as a style dependency.

Merges [30832] to the 4.1 branch.

Props obenland, SergeyBiryukov.
Fixes #30678.

#15 @johnbillion
6 years ago

  • Keywords commit fixed-major removed
Note: See TracTickets for help on using tickets.