[#11412] - system/sef plugin background image parser regexp bug
- Closed
- 3 Aug 2016
- Medium
- Build: staging
- # 11412
- Diff
- brianteeman:regex
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #7267 .
Bug
The problem is that using images in the style attributes for background images and the image urls with a full url:
<div style="background-image: url('http://test.org/test.png')"></div>
The regexp is in plugins/system/sef/sef.php which used to find image urls in style attributes to fix them to the current site's relative path:
Test:
<div style="background-image: url('http://test.org/test.png')"></div>
Result:
<div style="background-image: url("/my/site/path/'http://test.org/test.png'")"></div>
Desired result: - (Regexp do not match this url.)
<div style="background-image: url('http://test.org/test.png')"></div>
Actually we need to exclude:
Single quotes
' '
Double quotes
" "
Also my initial suggestion does not work right for all cases
Thus yes this should be excluded
<div style="background-image: url('http://test.org/test.png')"></div>
But this should be included
<div style="background-image: url('images/test.png')"></div>
The check needs to go earilier at regexp at quotes checking:
[\'\"]
should be
([\'\"]|\&\#0?34;|\&\#0?39;)
@brianteeman
I ll test that it works for desired cases and commit to other PR ?
- also another reason for adding this to the other PR, is that the other PR will output the original type of quotes:
'
"
'
'
"
"
When you update your PR I will close this - thanks
On 3 August 2016 at 14:46, Georgios Papadakis notifications@github.com
wrote:
Actually we need to exclude:
Single quotes' '
Double quotes
" "
Also my initial suggestion does not work right for all cases
Thus yes this should be excluded
But this should be included
The check needs to go earilier at regexp at quotes checking:
[\'\"]
should be
([\'\"]|�?34;|�?39;)
@brianteeman https://github.com/brianteeman
I ll test that it works for desired cases and commit to other PR ?
- also another reason for adding this to the other PR, is that the other PR will output the original type of quotes:
'
"
'
'
"
"—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11412 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8cZHSY1GMSQUyz34VzgRBCc4QkxMks5qcJu6gaJpZM4JboBn
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
i have updated the other PR to add cases like:
style='background-image:url("...");'
style="background-image:url('...');"
style='background-image:url("...");'
style="background-image:url('...');"
So you can close this PR, i tried to make some easier and more complete testing instructions too
brian isn't this the same as #11266 ?