[#16452] - Fallback for keepalive js
- Fixed in Code Base
- 13 Jun 2017
- Medium
- Build: staging
- # 16452
- Diff
- andrepereiradasilva:patch-20
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
- Failure continuous-integration/appveyor/pr AppVeyor build failed Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #16375 (only keepalive fallback part).
Summary of Changes
The keepalive js fallback is not working correctly when there is no system.keepalive json. This PR intends to add better fallback for those wild scenarios.
Testing Instructions
Pre-requisites:
- Use latest joomla staging with a login module in the frontend (so keepalive is called)
- In global config set joomla session to 1 minute (for faster test) and System Cache to "ON - Progressive caching"
Reproducing the issue:
- Clear joomla cache
- Go to frontend - not logged, reload the page two times (Ctrl+F5)
- Confirm there is a js error in the Chrome Debug Console "Console" tab (F12)
Test if proposed keepalive fallback works:
- Apply patch #16488 (if not merged) and them this patch
- Repeat steps in "Reproducing the issue", but now confirm there is no js error in the Chrome Debug Console "Console" tab (F12)
- Wait one minute and also confirm keepalive in called correctly in Chrome Debug Console "Network" tab
Expected result
Keepalive should always work even if there is no system.keepalive json.
Actual result
Keepalive does not work in this scenario because of a js error.
Documentation Changes Required
@dgt41 @Fedik can you also check this one and confirm there is no problem in adding the joomla install uri paths trough script options in behavior.core, ie, all the pages that use the core bahaviour (this would also be used in #15529).
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries JavaScript |
| Labels |
Added:
?
|
||
After PR, I still get a js error:
Uncaught TypeError: Cannot read property 'system.keepalive' of null
at Object.e.getOptions (core.js?449d852…:1)
at HTMLDocument.<anonymous> (keepalive.js?449d852…:1)
@andrepereiradasilva I guess there still bug with Joomla.loadOptions
there
if (!Joomla.optionsStorage) {
Joomla.optionsStorage = options;
}need to be:
if (!Joomla.optionsStorage) {
Joomla.optionsStorage = options || {};
}Can you add it here or I do new PR?
@dgt41 I thought that is your PR
we can update, after this one will be merged
another strange thing,
with this patch, if cache is enabled, the core.js loaded but there no any <script type="application/json" class="joomla-script-options"> in the head.
maybe it somehow related to JCache::set(get)Workarounds ?
upd. debug off, and only on "Progressive caching"
Can you add it here or I do new PR?
please do a PR i prefer you do that Joomla.optionsStorage part - i will test it so it can be RTC and merged fast (hope @dgt41 tests it too
this one stays in standby until your is merged
another strange thing,
with this patch, if cache is enabled, the core.js loaded but there no any <script type="application/json" class="joomla-script-options"> in the head.
maybe it somehow related to JCache::set(get)Workarounds ?
that is the first problem of the issue that originated this PR see #16375 (comment)
But that's another issue not related to js.
@andrepereiradasilva there is pull #16488
which should fix:
Uncaught TypeError: Cannot read property 'system.keepalive' of null
at Object.e.getOptions (core.js?449d852…:1)
at HTMLDocument.<anonymous> (keepalive.js?449d852…:1)
Current pull should work after #16488
I am still getting the js error after both PRs are applied in the frontend.
Uncaught TypeError: Cannot read property 'system.keepalive' of null
at Object.Joomla.getOptions (core.js?cf50a49…:1)
at HTMLDocument.<anonymous> (keepalive.js?cf50a49…:1)
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16452.
I have tested this item
works as described, with #16488
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16452.
| Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-13 14:51:11 |
| Closed_By | ⇒ | rdeutz |
@andrepereiradasilva I did not tested, but looks good to me