Bug #12396
closedReview, CGL issues, security
100%
Description
during a review I found some CGL and general coding style issues, sorry for the late review though...
Most of the stuff should be easy search and replace...
Blockers¶
They must be fixed until release.
=> See these subtasks:
#12410: Fix ChangeLog
#12411: Fix Copyright Notices
#12412: XSS issue in tx_linkvalidator_tasks_ValidateAdditionalFieldProvider
#12414: OOP principle of data encapsulation violated in tx_linkvalidator_tasks_Validate
#12416: tx_linkvalidator_tasks_Validate: Missing public / protected declarations
All blockers are fixed now.
The following things are no blockers.
UI and usabilty issues¶
- why are there two buttons, one should be enough
- the intro text at the top is nice when using the link checker the first time but should rather go into CSH instead of being there all the time
- was the UI approved by the UI team?
- remove the selection of (I guess) what links to show. Here, less is more. List all links independently of what checks have been hooked in.
- use the rendered FE output to check for dead links instead of just checking the DB, this is more reliable (ok for 4.5, should be fixed in later versions)
- might also use the reference index
=> Patrick explained those issues on the list.
For possible improvements see #12447.
General stuff¶
ext_tables.sql
=> Fixed in the subtasks
#12421: Rename tx_linkvalidator_links to tx_linkvalidator_link
#12422: Do not use abbreviations for field names (recuid, recpid)
#12423: Use underscores to separate words in field names
ext_localconf.php
See #12424
modfunc1
- #12425: Rename modfunc1
- #12426: Rename class names (naming conventions)
- #12428: Naming of member properties: Use camelCase instead of underscores: $this->search_level
- #12427: Rename draw*() methods to render*()
- #12431: putting labels into members, is this necessary? (it's at least very uncommon): $this->firstSteps
- #12429: Do not use private accessors
- Not possible: why have initialize()? Would __construct() work?
- #12647: No API used to build the links in renderTableRow()
- #12648: the way the table is rendered does not necessarily follow TYPO3 API, check the scheduler for example. This is ok for 4.5 though, but should be changed later .
classes
See- #12426: Rename class names (naming conventions)
tx_linkvalidator_processing
- #12649: analyzeRecord() nesting too deep, too long method / too many lines. try to split up (not required for 4.5)
tx_linkvalidator_tasks_Validate
- #12650: exec() is too long (ok for 4.5)