Bug #72931

Epic #65814: Make Indexed search extbase plugin shine

Indexed search with strict standards

Added by Benno Flory over 3 years ago. Updated 12 months ago.

Status:
Closed
Priority:
Must have
Category:
Indexed Search
Start date:
2016-01-25
Due date:
% Done:

100%

Estimated time:
1.00 h
TYPO3 Version:
7
PHP Version:
7.0
Tags:
Complexity:
medium
Is Regression:
No
Sprint Focus:

Description

When posting the search form with an empty search field, I get 2 strict standard warnings (although error reporting is switched off):

Strict Standards: Declaration of TYPO3\CMS\IndexedSearch\Controller\SearchFormController::pi_list_browseresults() should be compatible with TYPO3\CMS\Frontend\Plugin\AbstractPlugin::pi_list_browseresults($showResultCount = 1, $tableParams = '', $wrapArr = Array, $pointerName = 'pointer', $hscText = true, $forceOutput = false) in /var/www/vhosts/az-direct.ch/httpdocs/typo3_src-6.2.15/typo3/sysext/indexed_search/Classes/Controller/SearchFormController.php on line 2423

Strict Standards: Only variables should be assigned by reference in /var/www/vhosts/az-direct.ch/httpdocs/typo3_src-6.2.15/typo3/sysext/indexed_search/Classes/Controller/SearchFormController.php on line 537

My (probably dirty, but workung) solutions:

Old lines 1589...: * @param boolean $showResultCount Show result count * @param string $addString String appended to "displaying results..." notice. * @param string $addPart String appended after section "displaying results... * @param string $freeIndexUid List of integers pointing to free indexing configurations to search. -1 represents no filtering, 0 represents TYPO3 pages only, any number above zero is a uid of an indexing configuration! * @return string HTML output * @todo Define visibility
*/
public function pi_list_browseresults($showResultCount = TRUE, $addString = '', $addPart = '', $freeIndexUid = -1) {

New lines 1589...: * @param integer $showResultCount Show result count * @param string $addString String appended to "displaying results..." notice. * @param array $addParts Array containing string appended after section "displaying results... * @param string $freeIndexUid List of integers pointing to free indexing configurations to search. -1 represents no filtering, 0 represents TYPO3 pages only, any number above zero is a uid of an indexing configuration! * @param boolean $strictStandardsDummy1 - See BUGFIXES above * @param boolean $strictStandardsDummy2 - See BUGFIXES above * @return string HTML output * @todo Define visibility
*/
public function pi_list_browseresults($showResultCount = 1, $addString = '', $addParts = array(), $freeIndexUid = '-1', $strictStandardsDummy1 = TRUE, $strictStandardsDummy2 = TRUE) {

Old line 1638: $addPart .= '
New line 1638: $addParts[] = '

Old line 1656: . $addPart . '</div>';
New line 1656: . implode('', $addParts) . '</div>';

Old line 537: if ($hookObj = &$this->hookRequest('getResultRows_SQLpointer')) {
New line 537: if ($hookObj = $this->hookRequest('getResultRows_SQLpointer')) {

My versions: TYPO3 CMS 6.2.15 / PHP 5.4.36


Related issues

Duplicated by TYPO3 Core - Bug #75457: indexed_search not compatible with PHP7 Closed 2016-04-08

Associated revisions

Revision c39ea264 (diff)
Added by Tymoteusz Motylewski over 3 years ago

[!!!][BUGFIX] Make indexed search plugin PHP7 compatible

Rename SearchFormController::pi_list_browseresults as it had different
signature than defined in parent AbstractPlugin and there is no
non breaking way to make it PHP7 compatible.

Resolves: #72931
Releases: 7.6, master
Change-Id: I3dc36386a3866b5ca87d48d2869c21b9a37fbfe3
Reviewed-on: https://review.typo3.org/47349
Reviewed-by: Xavier Perseguers <>
Reviewed-by: Morton Jonuschat <>
Tested-by: Morton Jonuschat <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

Revision c326bb1e (diff)
Added by Tymoteusz Motylewski over 3 years ago

[!!!][BUGFIX] Make indexed search plugin PHP7 compatible

Rename SearchFormController::pi_list_browseresults as it had different
signature than defined in parent AbstractPlugin and there is no
non breaking way to make it PHP7 compatible.

Resolves: #72931
Releases: 7.6, master
Change-Id: I3dc36386a3866b5ca87d48d2869c21b9a37fbfe3
Reviewed-on: https://review.typo3.org/47416
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

History

#1 Updated by Benno Flory over 3 years ago

Sorry, with better formatting:
Old lines 1589...:
  • @param boolean $showResultCount Show result count
  • @param string $addString String appended to "displaying results..." notice.
  • @param string $addPart String appended after section "displaying results...
  • @param string $freeIndexUid List of integers pointing to free indexing configurations to search. -1 represents no filtering, 0 represents TYPO3 pages only, any number above zero is a uid of an indexing configuration!
  • @return string HTML output
  • @todo Define visibility
    */
    public function pi_list_browseresults($showResultCount = TRUE, $addString = '', $addPart = '', $freeIndexUid = -1) {
New lines 1589...:
  • @param integer $showResultCount Show result count
  • @param string $addString String appended to "displaying results..." notice.
  • @param array $addParts Array containing string appended after section "displaying results...
  • @param string $freeIndexUid List of integers pointing to free indexing configurations to search. -1 represents no filtering, 0 represents TYPO3 pages only, any number above zero is a uid of an indexing configuration!
  • @param boolean $strictStandardsDummy1 - See BUGFIXES above
  • @param boolean $strictStandardsDummy2 - See BUGFIXES above
  • @return string HTML output
  • @todo Define visibility
    */
    public function pi_list_browseresults($showResultCount = 1, $addString = '', $addParts = array(), $freeIndexUid = '-1', $strictStandardsDummy1 = TRUE, $strictStandardsDummy2 = TRUE) {

Additionally:
Old lines 680/681:
$browseBox1 = $this->pi_list_browseresults(1, $addString, $this->printResultSectionLinks(), $freeIndexUid);
$browseBox2 = $this->pi_list_browseresults(0, '', '', $freeIndexUid);

New lines 680/681:
$browseBox1 = $this->pi_list_browseresults(1, $addString, array($this->printResultSectionLinks()), $freeIndexUid);
$browseBox2 = $this->pi_list_browseresults(0, '', array(), $freeIndexUid);

#2 Updated by Riccardo De Contardi over 3 years ago

  • Parent task set to #65814

#3 Updated by Morton Jonuschat over 3 years ago

  • Target version changed from 6.2.18 to Candidate for patchlevel

#4 Updated by Christian Kuhn over 3 years ago

confirmed.

we also need to fix this now with php 7 on core v7 and master, since e_strict was mapped to other error_types with php 7, so this is now a solid warning.

#5 Updated by Christian Kuhn over 3 years ago

default 6.2 configuration is to suppress e_strict and 6.2 is not compatible with php 7 anyway.

so: do NOT fix in 6.2, but do fix for 7.6 and master.

#6 Updated by Christian Kuhn over 3 years ago

  • TYPO3 Version changed from 6.2 to 7
  • PHP Version changed from 5.4 to 7.0

#7 Updated by Tymoteusz Motylewski over 3 years ago

  • Assignee set to Tymoteusz Motylewski

I'm wondering what is the best way to solve it.

1. Simplest is to add dummy parameters, to make the count match with parent method as Benno suggested.

However this means that the method signature will stay different from the logical point of view - indexed search params have different meaning then one from AbstractPlugin.

2. other solution would be to rename pi_list_browseresults to something else. Indexed Search is not calling parent::pi_list_browseresults anyway.

As both solutions are breaking I think we should go for the 2nd.

#8 Updated by Gerrit Code Review over 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#9 Updated by Gerrit Code Review over 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#10 Updated by Gerrit Code Review over 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#11 Updated by Gerrit Code Review over 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#12 Updated by Gerrit Code Review over 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#13 Updated by Gerrit Code Review over 3 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47349

#14 Updated by Gerrit Code Review over 3 years ago

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47416

#15 Updated by Tymoteusz Motylewski over 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#16 Updated by Steffen Liebig over 3 years ago

Hi there,

in our installation, the mistake is still valid:

#1: PHP Warning: Declaration of TYPO3\CMS\IndexedSearch\Controller\SearchFormController::pi_list_browseresults($showResultCount = true, $addString = '', $addPart = '', $freeIndexUid = -1) should be compatible with TYPO3\CMS\Frontend\Plugin\AbstractPlugin::pi_list_browseresults($showResultCount = 1, $tableParams = '', $wrapArr = Array, $pointerName = 'pointer', $hscText = true, $forceOutput = false) in /home/www/typo3751/typo3_src-7.6.4/typo3/sysext/indexed_search/Classes/Controller/SearchFormController.php line 2504

I tried to apply the given changes, but this only transfers the error message from line 2504 to line 28 (where's only mentioned what the file is supposed to do). Curiosity show in it: I have to leave out the comment "former class xyz". If I don't, the error message occurs related to another (a 3rd) line. The explanation seems to be interpreted as an order - though the line is surely outcommented as an explanation for the following code.

Technical stuff:
Typo3 7.6.4
PHP 7.0.4
indexed_sarch from updated 7.5.1/7.6.2-installation
own xclasses: none

Macina_searchbox works fine - well, it's another module of course, but the output is on the same page as it should be with indexed_search. Doesn't matter for the error, does it ?

(Maybe) interesting info:
- Our 7.6.4 is an updated testing installation where the 7.5.1-installation was done clearly by our provider's installation routine while the rest worked properly after a core update in the installation tool. Additionally, I used a db-dump of the 6.2.14-site and ran the install toll doing necessary db updates. I still needed the old T3 cause I had to test the 7.x first. Therefore and cause I'm not really familar with consoles I didn't use a new core via changing the "never-found" symlinks. Too confusing and not working, a clean installation by routine is easier g. Maybe this affected indexed_search/T3 7.x ?!
- Our live-site works under 6.2.14/PHP 7 - the backend shows error messages, but I can handle the system. In this installation, no problem with indexed_search.

Under another 6.2.14(-testing) installation, the module shows up the search text filed, but doesn't find any results (tried a few TS-configuration hints, but it didn't work - I restored the old state).

Cu, Steffen

#17 Updated by Wouter Wolters over 3 years ago

Can you try a 7.6.5 installation and make sure you clear the opcode caches in the install tool if available?

#18 Updated by Tymoteusz Motylewski over 3 years ago

I have just tested indexed_search on 7.6.6-dev and it's working fine on PHP 7

#19 Updated by Steffen Liebig over 3 years ago

Trying a core update, I got a general error which is not defined furthermore. Our provider has 7.6.5 so I will try to make clear about this error and update. Back asap...

#20 Updated by Steffen Liebig over 3 years ago

Strange: i just found out that meanwhile the macina searchbox is leading to the same error (line 28), but don't know if that's a result from clearing caches. Via install tool, I cleared all caches and the log table, op code cache button not available (maybe it "returns" after core update ?!). Then, I run a database and ax extension check (as awaited - both are ok).
Even more strange: apache tells me about two errors - not existing files "ExtensionCompatibilityTesterErrors.json" and "robots.txt" (I know what a robots.txt is - maybe it belongs to the standard system and I just don't really use it...but what about this ...testererrors.json ?).

#21 Updated by Steffen Liebig over 3 years ago

Update: our provider replaced the 7.6.4-core by a 7.6.5. At first (and second) glance i can report that search is working again :-).

This has not fixed the general error which still occurs when clicking core update in install tool, but this should be a topic to be posted somewhere else ?!

Would be glad about an answer to this question - best by mail I think. We can close here for the moment :-).

#22 Updated by Benni Mack 12 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF