Fix minifyCSS() for Quoted URLs in Media.php #1516

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@Xperos
Contributor

Xperos commented Mar 8, 2014

This fix allows for URLs to be enclosed in quotes. The existing code tests for the opening quote in the wrong place, it should be at the beginning of the URL.

Fix minifyCSS() for Quoted URLs in Media.php
This fix allows for URLs to be enclosed in quotes. The existing code tests for the opening quote in the wrong place, it should be at the beginning of the URL.

@gRoussac gRoussac self-assigned this Nov 21, 2014

@jnadaud jnadaud added the bug fix label Mar 23, 2015

@jnadaud

This comment has been minimized.

Show comment
Hide comment
@jnadaud

jnadaud Apr 10, 2015

Contributor

Hi,

thank you for your contribution, can you please sync your fork and check if this PR is still relevant ?

Best regards.

Contributor

jnadaud commented Apr 10, 2015

Hi,

thank you for your contribution, can you please sync your fork and check if this PR is still relevant ?

Best regards.

@Xperos

This comment has been minimized.

Show comment
Hide comment
@Xperos

Xperos Apr 11, 2015

Contributor

Having reviewed the up to date repository I believe that a change to minifyCSS() is still warranted. The proposed modification will only call replaceByAbsoluteURL() where data:, http: or https: is not specified. This removed unnecessary calls to replaceByAbsoluteURL(). The modified regex is also slightly more efficient.

In addition replaceByAbsoluteURL() has been modified to correctly handle absolute addresses i.e. those beginning with /

I have created a new pull request #2867 for this and marking #1516 closed.

Contributor

Xperos commented Apr 11, 2015

Having reviewed the up to date repository I believe that a change to minifyCSS() is still warranted. The proposed modification will only call replaceByAbsoluteURL() where data:, http: or https: is not specified. This removed unnecessary calls to replaceByAbsoluteURL(). The modified regex is also slightly more efficient.

In addition replaceByAbsoluteURL() has been modified to correctly handle absolute addresses i.e. those beginning with /

I have created a new pull request #2867 for this and marking #1516 closed.

@Xperos Xperos closed this Apr 11, 2015

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