Project

General

Profile

Actions

Bug #12396

closed

Review, CGL issues, security

Added by Ingo Renner over 13 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Linkvalidator
Target version:
-
Start date:
2011-01-20
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
TYPO3 Version:
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

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)

Subtasks 5 (0 open5 closed)

Bug #12411: Fix Copyright NoticesClosedMichael Miousse2011-01-20

Actions
Bug #12414: OOP principle of data encapsulation violated in tx_linkvalidator_tasks_ValidateClosedMichael Miousse2011-01-20

Actions
Bug #12422: ext_tables.sql: Do not use abbreviations for field names (recuid, recpid)ClosedChris topher2011-01-21

Actions
Bug #12429: do not use private accessorsClosedMichael Miousse2011-01-21

Actions
Bug #12431: putting labels into members, is this necessary (it's at least very uncommon): $this->firstStepsClosedMichael Miousse2011-01-21

Actions
Actions

Also available in: Atom PDF