From 30c4f7d40d2f494d70f687a10fd386aed9fada3d Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Tue, 27 Feb 2024 23:57:44 -0300 Subject: [PATCH] Avoid loading theme fonts again and assume they were already resolved by the font face resolver. (#59421) * Avoid loading theme fonts again and assume they were already resolved by the font face resolver. This allows us to remove some related to guess the url of theme fonts. * fix comment text Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> --------- Co-authored-by: matiasbenedetto Co-authored-by: peterwilsoncc Co-authored-by: youknowriad Co-authored-by: t-hamano Co-authored-by: swissspidy --- .../font-library-modal/context.js | 12 +-------- .../font-library-modal/utils/index.js | 14 ++++++++--- .../test/getDisplaySrcFromFontFace.spec.js | 25 ++++++------------- 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/context.js b/packages/edit-site/src/components/global-styles/font-library-modal/context.js index 35276b0ad8b2b..8da33192859f1 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/context.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/context.js @@ -162,16 +162,6 @@ function FontLibraryProvider( { children } ) { // Demo const [ loadedFontUrls ] = useState( new Set() ); - // Theme data - const { site, currentTheme } = useSelect( ( select ) => { - return { - site: select( coreStore ).getSite(), - currentTheme: select( coreStore ).getCurrentTheme(), - }; - } ); - const themeUrl = - site?.url + '/wp-content/themes/' + currentTheme?.stylesheet; - const getAvailableFontsOutline = ( availableFontFamilies ) => { const outline = availableFontFamilies.reduce( ( acc, font ) => { const availableFontFaces = @@ -416,7 +406,7 @@ function FontLibraryProvider( { children } ) { // If the font doesn't have a src, don't load it. if ( ! fontFace.src ) return; // Get the src of the font. - const src = getDisplaySrcFromFontFace( fontFace.src, themeUrl ); + const src = getDisplaySrcFromFontFace( fontFace.src ); // If the font is already loaded, don't load it again. if ( ! src || loadedFontUrls.has( src ) ) return; // Load the font in the browser. diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js index 011f09b12a841..1458b47cd010a 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js @@ -121,7 +121,13 @@ export async function loadFontFaceInBrowser( fontFace, source, addTo = 'all' ) { } } -export function getDisplaySrcFromFontFace( input, urlPrefix ) { +/** + * Retrieves the display source from a font face src. + * + * @param {string|string[]} input - The font face src. + * @return {string|undefined} The display source or undefined if the input is invalid. + */ +export function getDisplaySrcFromFontFace( input ) { if ( ! input ) { return; } @@ -132,9 +138,9 @@ export function getDisplaySrcFromFontFace( input, urlPrefix ) { } else { src = input; } - // If it is a theme font, we need to make the url absolute - if ( src.startsWith( 'file:.' ) && urlPrefix ) { - src = src.replace( 'file:.', urlPrefix ); + // It's expected theme fonts will already be loaded in the browser. + if ( src.startsWith( 'file:.' ) ) { + return; } if ( ! isUrlEncoded( src ) ) { src = encodeURI( src ); diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getDisplaySrcFromFontFace.spec.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getDisplaySrcFromFontFace.spec.js index 9c6235443a099..3cbdc0283f1a9 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getDisplaySrcFromFontFace.spec.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getDisplaySrcFromFontFace.spec.js @@ -21,33 +21,22 @@ describe( 'getDisplaySrcFromFontFace', () => { ); } ); - it( 'makes URL absolute when it starts with file:. and urlPrefix is given', () => { - const input = 'file:./font1'; - const urlPrefix = 'http://example.com'; - expect( getDisplaySrcFromFontFace( input, urlPrefix ) ).toBe( - 'http://example.com/font1' - ); - } ); - - it( 'does not modify URL if it does not start with file:.', () => { - const input = [ 'http://some-other-place.com/font1' ]; - const urlPrefix = 'http://example.com'; - expect( getDisplaySrcFromFontFace( input, urlPrefix ) ).toBe( - 'http://some-other-place.com/font1' - ); + it( 'return undefined when the url starts with file:', () => { + const input = 'file:./theme/assets/font1.ttf'; + expect( getDisplaySrcFromFontFace( input ) ).toBe( undefined ); } ); it( 'encodes the URL if it is not encoded', () => { - const input = 'file:./assets/font one with spaces.ttf'; + const input = 'https://example.org/font one with spaces.ttf'; expect( getDisplaySrcFromFontFace( input ) ).toBe( - 'file:./assets/font%20one%20with%20spaces.ttf' + 'https://example.org/font%20one%20with%20spaces.ttf' ); } ); it( 'does not encode the URL if it is already encoded', () => { - const input = 'file:./font%20one'; + const input = 'https://example.org/fonts/font%20one.ttf'; expect( getDisplaySrcFromFontFace( input ) ).toBe( - 'file:./font%20one' + 'https://example.org/fonts/font%20one.ttf' ); } ); } );