Skip to content
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

The fix for Joomla progressive cache causing multiple duplicate javas… #20310

Merged
merged 1 commit into from May 20, 2019

Conversation

Projects
None yet
8 participants
@tgv604
Copy link
Contributor

commented May 7, 2018

…cript declarations

The issue has been described and discussed at
#5933
#2558
joomla/joomla-platform#673

In my case I had a problem with JCE mediabox plugin failing once a page has been cached, due to its init script getting duplicated up to 8 times.

This PR fixes the bugs in two lines that were meant to prevent duplication but the stristr() checks string inclusion in the wrong way: $sdata/$stdata here is the cached script declarations string so it has to be a haystack and be the first argument.

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

The fix for Joomla progressive cache causing multiple duplicate javas…
…cript declarations

The issue has been described and discussed at 
#5933
#2558
joomla/joomla-platform#673

In my case I had a problem with JCE mediabox plugin failing once a page has been cached, due to its init script getting duplicated up to 8 times. 

This PR fixes the bugs in two lines that were meant to prevent duplication but the stristr() checks string inclusion in the wrong way: $sdata/$stdata here is the cached script declarations string so it has to be a haystack and be the first argument.
@infograf768

This comment has been minimized.

Copy link
Member

commented May 7, 2018

@klas
Can you have a look?

@klas

This comment has been minimized.

Copy link
Contributor

commented May 7, 2018

Haven't tested, but so what @tgv604 proposes makes sense, newly added script should be needle and what is already in the storage (cache) should be haystack and documentations says that arguments order are stristr ( string $haystack , mixed $needle )

@Quy

This comment has been minimized.

Copy link
Contributor

commented May 7, 2018

I have tested this item successfully on eaa0fd2

Tested with JCE Mediabox.

Before PR:

h1, h2, h3, h4, h5, h6, .site-title {
        font-family: 'Open Sans', sans-serif;
}
h1, h2, h3, h4, h5, h6, .site-title {
        font-family: 'Open Sans', sans-serif;
}
JCEMediaBox.init({popup:{width:"",height:"",legacy:0,lightbox:0,shadowbox:0,resize:1,icons:1,overlay:1,overlayopacity:0.8,overlaycolor:"#000000",fadespeed:500,scalespeed:500,hideobjects:0,scrolling:"fixed",close:2,labels:{'close':'Close','next':'Next','previous':'Previous','cancel':'Cancel','numbers':'{$current} of {$total}'},cookie_expiry:"",google_viewer:0},tooltip:{className:"tooltip",opacity:0.8,speed:150,position:"br",offsets:{x: 16, y: 16}},base:"/joomla-cms-staging/",imgpath:"plugins/system/jcemediabox/img",theme:"standard",themecustom:"",themepath:"plugins/system/jcemediabox/themes",mediafallback:0,mediaselector:"audio,video"});

JCEMediaBox.init({popup:{width:"",height:"",legacy:0,lightbox:0,shadowbox:0,resize:1,icons:1,overlay:1,overlayopacity:0.8,overlaycolor:"#000000",fadespeed:500,scalespeed:500,hideobjects:0,scrolling:"fixed",close:2,labels:{'close':'Close','next':'Next','previous':'Previous','cancel':'Cancel','numbers':'{$current} of {$total}'},cookie_expiry:"",google_viewer:0},tooltip:{className:"tooltip",opacity:0.8,speed:150,position:"br",offsets:{x: 16, y: 16}},base:"/joomla-cms-staging/",imgpath:"plugins/system/jcemediabox/img",theme:"standard",themecustom:"",themepath:"plugins/system/jcemediabox/themes",mediafallback:0,mediaselector:"audio,video"});

After PR, no duplicates


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20310.

@viocassel

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

I have tested this item successfully on eaa0fd2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20310.

@Quy

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/20310.

@joomla-cms-bot joomla-cms-bot added the RTC label May 18, 2019

@HLeithner HLeithner merged commit 014bf30 into joomla:staging May 20, 2019

5 checks passed

Hound No violations found. Woof!
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@HLeithner

This comment has been minimized.

Copy link
Member

commented May 20, 2019

thx

@joomla-cms-bot joomla-cms-bot removed the RTC label May 20, 2019

@HLeithner HLeithner added this to the Joomla 3.9.7 milestone May 20, 2019

tecpromotion added a commit to tecpromotion/joomla-cms that referenced this pull request May 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.