processwire / processwire-issues

ProcessWire issue reports.
44 stars 2 forks source link

MarkupQA incorrectly flags pages that contain the admin path in their path #1216

Closed schwarzdesign closed 4 years ago

schwarzdesign commented 4 years ago

Short description of the issue

We're getting incorrect warnings when saving a page that contains links to a particular page. The issue is that MarkupQA checks if a URL points to an admin page uses strpos to determine if a path leads to an admin page, which produces incorrect results if the admin path appears somewhere else inside the path of a link being checked.

Expected behavior

The warning should only be triggered when the admin URL appears at the very start of the path of the link being checked.

Actual behavior

Instead of the default admin path /processwire/, we use the more generic /cms/. We also have a regular page titled CMS in our page tree, with path /kompetenzen/cms/.

I traced the issue to this part of MarkupQA::wakeupLinks. In our case, the variables contain the following:

The conditional check incorrectly returns true in this case, because the $adminPath is a substring of the $livePath.

Screenshots/Links that demonstrate the issue

incorrect-warnings

Suggestion for a possible fix

Instead of just checking if strpos returns any value, it should check if strpos returns 0, since the a link to an admin page would begin with the admin path.

// current behavior, incorrect result
} else if(strpos($livePath, $adminPath) !== false) {

// better, will not produce an error in the case described above
} else if(strpos($livePath, $adminPath) === 0) {

Steps to reproduce the issue

Create a regular page as a child of another page where the path is identical with the admin path. Then link to that page inside a CK Editor field from another page. The warning will appear after saving.

Setup/Environment

ryancramerdesign commented 4 years ago

Thanks @schwarzdesign I have added your suggested fix.