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

Apache rules for .woff2 font files #7948

Merged
merged 2 commits into from Jun 21, 2017

Conversation

Projects
None yet
4 participants
@axometeam
Contributor

axometeam commented May 31, 2017

Questions Answers
Branch? 1.6.1.x
Description? Apache optimisation rules don't include woff2 files, but these are integrated in the default theme (font awesome from bootstrap 3)
Type? improvement
Category? FO
BC breaks? no
Deprecations? no
How to test? Install a fresh 1.6.1.13, with the default theme. Configure a media server (eg. site on localhost and media server on media-server-localhost, both pointing to 127.0.0.1). Watch the Chrome console and see an "Access-Control-Allow-Origin" error for woff2 files

@axometeam axometeam changed the title from [+] FO: apache rules for .woff2 font files to FO: apache rules for .woff2 font files May 31, 2017

@axometeam axometeam changed the title from FO: apache rules for .woff2 font files to [+] FO: apache rules for .woff2 font files May 31, 2017

@Quetzacoalt91

Already available on 1.7.

See

@@ -2509,9 +2509,10 @@ public static function generateHtaccess($path = null, $rewrite_settings = null,
fwrite($write_fd, "AddType application/vnd.ms-fontobject .eot\n");
fwrite($write_fd, "AddType font/ttf .ttf\n");
fwrite($write_fd, "AddType font/otf .otf\n");
fwrite($write_fd, "AddType font/woff2 .woff2\n");

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Jun 1, 2017

Member

Aren't we supposed to set

AddType  application/font-woff2  .woff2

?

@Quetzacoalt91

Quetzacoalt91 Jun 1, 2017

Member

Aren't we supposed to set

AddType  application/font-woff2  .woff2

?

This comment has been minimized.

@nsorosac

nsorosac Jun 1, 2017

Contributor

If we refer to the w3c spec, it should be font/woff2

source: https://dev.w3.org/webfonts/WOFF2/spec/#IMT

@nsorosac

nsorosac Jun 1, 2017

Contributor

If we refer to the w3c spec, it should be font/woff2

source: https://dev.w3.org/webfonts/WOFF2/spec/#IMT

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Jun 1, 2017

Member

Then there is a mistake to fix in the Tools.php of the develop branch.

@Quetzacoalt91

Quetzacoalt91 Jun 1, 2017

Member

Then there is a mistake to fix in the Tools.php of the develop branch.

@Quetzacoalt91 Quetzacoalt91 changed the title from [+] FO: apache rules for .woff2 font files to Apache rules for .woff2 font files Jun 1, 2017

@nsorosac

This comment has been minimized.

Show comment
Hide comment
@nsorosac

nsorosac Jun 1, 2017

Contributor

Already available on 1.7, but since it's a bugfix about the 1.6.1 branch, shouldn't you also fix it ?

The 1.6.1 default theme uses bootstrap 3, so it should be fixed; don't forget that the vast majority of PS shops are still running the 1.6 to 1.6.1 versions ;-)

Contributor

nsorosac commented Jun 1, 2017

Already available on 1.7, but since it's a bugfix about the 1.6.1 branch, shouldn't you also fix it ?

The 1.6.1 default theme uses bootstrap 3, so it should be fixed; don't forget that the vast majority of PS shops are still running the 1.6 to 1.6.1 versions ;-)

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Jun 1, 2017

Member

You're totally mistaken on my previous message.
If I didn't want to merge it on PrestaShop 1.6, I would have already closed it.

As you can see, it is still opened, and you even have something to fix. It was just related to the 1.7 fixes in order to make it quickly merged.

Member

Quetzacoalt91 commented Jun 1, 2017

You're totally mistaken on my previous message.
If I didn't want to merge it on PrestaShop 1.6, I would have already closed it.

As you can see, it is still opened, and you even have something to fix. It was just related to the 1.7 fixes in order to make it quickly merged.

@maximebiloe

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Jun 1, 2017

Contributor

On 1.6, 1.7.0.xand 1.7.1.x branches, we only merge before packaging a new version. Don't be surprised if your PR is not merge right away.

As @Quetzacoalt91 said, it was not closed, so we're interested in your suggestion. When you'll have made the requested changes, we'll milestone your PR.

Regards

Contributor

maximebiloe commented Jun 1, 2017

On 1.6, 1.7.0.xand 1.7.1.x branches, we only merge before packaging a new version. Don't be surprised if your PR is not merge right away.

As @Quetzacoalt91 said, it was not closed, so we're interested in your suggestion. When you'll have made the requested changes, we'll milestone your PR.

Regards

@maximebiloe maximebiloe added this to the 1.6.1.15 milestone Jun 12, 2017

@maximebiloe maximebiloe merged commit d2a0da8 into PrestaShop:1.6.1.x Jun 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maximebiloe

This comment has been minimized.

Show comment
Hide comment
@maximebiloe

maximebiloe Jun 21, 2017

Contributor

Thanks @axometeam

Contributor

maximebiloe commented Jun 21, 2017

Thanks @axometeam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment