Project

General

Profile

Actions

Bug #10989

closed

Anchors in internal links are incorrectly treated as pages

Added by Joh. Feustel over 13 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
Must have
Assignee:
Category:
Linkvalidator
Target version:
-
Start date:
2010-11-19
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

try adding a link to a content element like 1#123
This will end up with a message like

http://domain/index.php?id=123     "Page doesn't exist." 

regardless if there is a content element 123 on page 1 or not.
This is because the check is determined by the type "db" and does not take "recordRef" into account which is "pages:1" or "tt_content:123"

This is a problem since it adds meaningless messages if a user adds a single working link to a content element.


Files

10989.diff (13.6 KB) 10989.diff Chris topher, 2011-01-12 15:57
10989_v2.diff (17.4 KB) 10989_v2.diff Michael Miousse, 2011-01-13 18:16
10989_v3.diff (16.9 KB) 10989_v3.diff Chris topher, 2011-01-15 18:45
10989_v4.diff (16.7 KB) 10989_v4.diff Chris topher, 2011-01-15 20:22
10989_v5.diff (16.8 KB) 10989_v5.diff Michael Miousse, 2011-01-17 15:57
Actions #1

Updated by Tolleiv Nietsch over 13 years ago

  • Status changed from New to Accepted
Actions #2

Updated by Tolleiv Nietsch over 13 years ago

  • Assignee set to Tolleiv Nietsch
Actions #3

Updated by Chris topher about 13 years ago

  • Priority changed from Should have to Must have

Basically we only need to integrate a check in classes/...internal.php, if a page or a content element (=page with anchor) should be checked.
The code for pages already is there. Only the code for checking the content element has to be added (or integrated).

Jochen Rieger has already fixed that in linkchecker. This is the change. linkchecker is the code base of linkvalidator.
I think this solution has the disadvantage that it does not recognize the case, in which there is a problem with the page and with the element. Also it does not check, if an element really is on the page, to which it is linked. This misses elements, which were moved. So maybe not the ideal solution.

Actions #4

Updated by Chris topher about 13 years ago

  • File 10989.diff 10989.diff added
  • Status changed from Accepted to Needs Feedback
  • Assignee changed from Tolleiv Nietsch to Chris topher
  • % Done changed from 0 to 60

The attached patch is a first try to solve that.

I split up the functionality of checkLink(): Now checkLink basically only calculates the page id and content id and then calls the according functions to check page and content.
These functions are checkPage(), which checks for the linked page being OK and checkContent() which checks the content.

checkLink() in the end sets the errorParams and returns the response.

Please give me feedback!

Actions #5

Updated by Chris topher about 13 years ago

  • Status changed from Needs Feedback to Under Review
Actions #6

Updated by Michael Miousse about 13 years ago

I tested it and found a bug with the way we parse typolink.

i created those 2 link (first on is valid and second one is not) and all went rigth when i checked the links
but once i moved the content element 3 i had 3 link in my error report instead of 2

<a href="http://localhost/4.4.0/linkValidator/?id=2#3&quot; class="internal-link" title="Opens internal link in current window">testanchor valid</a>
<a href="http://localhost/4.4.0/linkValidator/?id=2#12345&quot; class="internal-link" title="Opens internal link in current window">testanchor invalide</a>

i investigated and found out that the database stored 3 link so the probleme was not in tx_linkvalidator_linkTypes_Internal
but in tx_linkvalidator_processing and that is when i realise that we parse $resultArray['elements'] to get the links event if we are in $spKey == 'typolink_tag' and that is why more links were saved in the database
so i took the liberty of changing the parsing a bit and here is a patch with the new parsing that corrects the error

feel free to give me your comments

Actions #7

Updated by Michael Miousse about 13 years ago

  • Status changed from Under Review to Needs Feedback
Actions #8

Updated by Chris topher about 13 years ago

Hi Michael,

here we go:

  • With your version 2 I cannot reproduce the problem, you wanted to fix (wrong number of results in table). :-)
  • Version 2 caused PHP warnings, because substr_count was used with $r['tokenID'] as substring. The problem was that $r['tokenID'] might be empty.
    I added an if (!empty($r['tokenID'])) {....} around these lines, which solves it for me.
  • The broken links we show were not completely correct: TYPO3 prepends each anchor with a "c", which I had forgotten. Fixed that.
  • I also cleaned up checkLink() and put all checks for the content element in checkContent()

I fixed that in version 3.

Actions #9

Updated by Chris topher about 13 years ago

Fixed in v4:
  • checkContent used 2 SQL queries, which basically were the same. This is now done in one query.
To Do:
  • One problem is still in (v3 and v4): After building a page with anchor, the same anchor is again used if the next link again is a page. (This leads to wrong error messages. The problem must be located in processing.php, where we construct "PAGE#cANCHOR".)
    We must still fix $wasPage somehow to show, if it really was the current link, which contained the anchor (and not another one). If it was not the current one, the anchor must be unset (which it does not get currently causing the problem).
Actions #10

Updated by Michael Miousse about 13 years ago

The problem was not in processing since this line check if the current anchor is in the current link:

if (substr_count($linkTags[$i], $r['tokenID'])) {

the error was in tx_linkvalidator_linkTypes_Internal
if ($anchor) {

// Check if the content element is OK.
$this->responseContent = $this->checkContent($page, $anchor, $softRefEntry, $reference);

}

$this->responseContent was never initialized to true so when no anchor was given, $this->responseContent had the la content check into it

i added $this->responseContent = TRUE; befor this call

worked for me

can you confirm?

Actions #11

Updated by Michael Miousse about 13 years ago

I will ask patrick to test it and if everyting is fine with him
Ill commit the patch but wont close the bug you will be able.
i need the changes from this patch in order to correct an other bug

Actions #12

Updated by Chris topher about 13 years ago

  • Subject changed from anchor links are treated as pages to Anchors in internal links are incorrectly treated as pages
  • Status changed from Needs Feedback to Under Review

Yes, seems to be working correctly now... One stupid line of code.... :-(

Finally committed in r42287.

I hope I integrated your changes correctly (your patch was somehow screwed up).

Could you check that again? If everyhing is OK, this one is done (finally). ;-)

Actions #13

Updated by Chris topher about 13 years ago

Michael Miousse wrote:

Ill commit the patch but wont close the bug you will be able.

First one! ;-)

Did I put your last changes in correctly?

Actions #14

Updated by Michael Miousse about 13 years ago

  • Status changed from Under Review to Resolved

Yep everything seem fine

good for a close

Actions #15

Updated by Chris topher about 13 years ago

  • % Done changed from 60 to 100

Cool!

Actions #16

Updated by Chris topher about 12 years ago

  • Status changed from Resolved to Closed
Actions #17

Updated by Michael Stucki over 10 years ago

  • Category set to Linkvalidator
Actions #18

Updated by Michael Stucki over 10 years ago

  • Project changed from 1510 to TYPO3 Core
  • Category changed from Linkvalidator to Linkvalidator
Actions

Also available in: Atom PDF