[#34482] - [4.0] Fix searchtool JS error
- Fixed in Code Base
- 12 Jun 2021
- Medium
- Build: 4.0-dev
- # 34482
- Diff
- joomdonation:fix_searchtools
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
Currently, if we use searchtools script but there are no filter fields for some reasons (it could happen with third party extensions like mine), there could be javascript errors in browser console like this:
Uncaught TypeError: Cannot read property 'forEach' of undefined
at Searchtools.checkActiveStatus (searchtools.js?2de576a4eb7e64e845e8e1a5fafd9f831d1aec5c:264)
at new Searchtools (searchtools.js?2de576a4eb7e64e845e8e1a5fafd9f831d1aec5c:194)
at HTMLDocument.onBoot (searchtools.js?2de576a4eb7e64e845e8e1a5fafd9f831d1aec5c:504)We have the code to check and make sure filter fields exist on other places like:
https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/js/searchtools.es6.js#L177
https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/js/searchtools.es6.js#L248
....
So I think it makes sense to have the same check in checkActiveStatus method, too.
Testing Instructions
I think this issue only happens for third party extensions only, so there is nothing to check from core. So we will have to rely on code review here.
@dgrammatiko Could you please take a look at this to see if the change is OK ?
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Repository NPM Change |
| Labels |
Added:
NPM Resource Changed
?
|
||
@dgrammatiko Could you test this PR by review? Thanks in advance.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34482.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34482.
Thanks @dgrammatiko .
@richard67 a more general note here, the [].slice.call(...) forces the HTMLElement.querySelectorAll() to return an Array. The difference is that HTMLElement.querySelectorAll() will return either:
undefinedis no element exists- a node list (which could be treated as an array)
So in the case that HTMLElement.querySelectorAll() returns undefined the code needs a conditional but if we have converted it to an array (in this case will be an empty array) the code won't break...
So in the case that
HTMLElement.querySelectorAll()returnsundefinedthe code needs a conditional but if we have converted it to an array (in this case will be an empty array) the code won't break...
@dgrammatiko Yes that's how I understand it.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-06-12 14:50:15 |
| Closed_By | ⇒ | Quy | |
| Labels |
Added:
?
|
||
Thanks!
I have tested this item✅ successfully on 7714b66
Code review and research what
[].slice.call(...)does, and noticed by review of diverse other PR's that we use it already elsewhere with success.This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34482.