User tests: Successful: Unsuccessful:
Speed up $input->remove() function, replace deprecated stuff.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Tags |
I'm not familiar with the tests, but I like to optimize the code.
The information about build "The Travis CI build failed" is incorrect.
Please check status of build at #9163
From a code-review point of view : The new code is equivalent to the old one and avoids a lot of unneeded cycles (speed optimization).
Milestone |
Added: |
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-01 08:51:24 |
Closed_By | ⇒ | roland-d |
I have merged this based on both code reviews and testing the PR myself.
Part of this change is a B/C break,
$source = $this->cleanTags($source); needs to remain as $source = $this->_cleanTags($source);
the underscore-methods need to remain in functions until the underscore-method is depreceated (4.0 in this case) @wilsonge fixed that B\C issue with 6e24ab4
@csthomas would you also consider making make this change at the Framework filter package?
@photodude I will push request on Framework filter package (without underscores)
Thank you for looking into this code. I personally have the feeling that JFilterInput is a weakness in Joomla, because it is EXTREMELY complex and I haven't been able to review it, when I looked into this a few years ago because of its complexity. I fear that there is a special string waiting somewhere out there, which is not properly handled by this class. Anyway, back to this PR
You are right that the code is executed more often than necessary. Can you move the variable assignment outside of the while condition? You could do a do {} while() instead, to run it at least once before checking the result.
It would be cool if we could get unittests for this class. Proper ones, that is.