Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#34986] - [4.0] Fix searchtools error checkActiveStatus

NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
30 Jul 2021

Pull Request for Issue # .

Summary of Changes

Look like [].slice.call does not accept undefined as a valid argument, so when there no filter container defined in search tool, this line of code https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/js/searchtools.es6.js#L289 throws some error in console.

Uncaught TypeError: Cannot convert undefined or null to object
    at slice (<anonymous>)
    at Searchtools.checkActiveStatus (searchtools.js?c804da20519ee3655f5ac687001b9f31321af327:262)
    at new Searchtools (searchtools.js?c804da20519ee3655f5ac687001b9f31321af327:194)
    at HTMLDocument.onBoot (searchtools.js?c804da20519ee3655f5ac687001b9f31321af327:504)

This PR prevents that error by adding a if check, similar with what we do in other places in that script, for example https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/js/searchtools.es6.js#L177

Testing Instructions

As this only happens on third party extensions (found this while testing my extensions), it's hard to call for end-users test. Hopefully code review would be enough. @dgrammatiko @Fedik Could you help?

avatar joomdonation joomdonation - open - 30 Jul 2021
avatar joomdonation joomdonation - change - 30 Jul 2021
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Jul 2021
Category ⇒ JavaScript Repository NPM Change
avatar dgrammatiko
dgrammatiko - comment - 30 Jul 2021

Probably just change the getFilterFields ?

    getFilterFields() {
      if (this.filterContainer) {
        return [].slice.call(this.filterContainer.querySelectorAll('select,input'));
      }
      return [];
    }

and also const els = [].slice.call(this.getFilterFields()); to const els = this.getFilterFields();

avatar joomdonation joomdonation - change - 30 Jul 2021
Labels Added: NPM Resource Changed ?
avatar joomdonation
joomdonation - comment - 30 Jul 2021

@dgrammatiko Yeah, better. And with that change, we can remove the if check before calling forEach in other places, thus optimize the process a bit :). Could you please check it again?

avatar dgrammatiko
dgrammatiko - comment - 30 Jul 2021

Approved

avatar richard67
richard67 - comment - 30 Jul 2021

@dgrammatiko If you mark your test result in the issue tracker (with a comment "Code review" or so maybe), then it will count ?

avatar joomdonation
joomdonation - comment - 30 Jul 2021

Help, please :). Otherwise I will ping you every time I hear from anyone reporting this bug with my extensions :D (joke).

avatar dgrammatiko dgrammatiko - test_item - 30 Jul 2021 - Tested successfully
avatar dgrammatiko
dgrammatiko - comment - 30 Jul 2021

I have tested this item ✅ successfully on bba304b

Code Review

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

avatar richard67 richard67 - test_item - 31 Jul 2021 - Tested successfully
avatar richard67
richard67 - comment - 31 Jul 2021

I have tested this item ✅ successfully on bba304b

Real test with one of @joomdonation 's extensions:

  • Issue reproduced.
  • Verified that PR fixes it.
  • Verified that search tools elsewhere still work as before.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34986.
avatar richard67 richard67 - change - 31 Jul 2021
Status Pending ⇒ Ready to Commit
avatar richard67
richard67 - comment - 31 Jul 2021

RTC

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

avatar richard67
richard67 - comment - 31 Jul 2021

@wilsonge As it's a bug fix I've set the 4.0 milestone. I hope this is ok.

avatar wilsonge wilsonge - close - 1 Aug 2021
avatar wilsonge wilsonge - merge - 1 Aug 2021
avatar wilsonge wilsonge - change - 1 Aug 2021
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2021-08-01 23:23:32
Closed_By ⇒ wilsonge
Labels Added: ?
avatar wilsonge
wilsonge - comment - 1 Aug 2021

Thanks!

Add a Comment

Login with GitHub to post a comment