Merge pull request #1083 from mateusz/fix-link-tracking

API Change broken link hihglighting to write to database.

_config/config.yml

@@ -4,6 +4,3 @@ LeftAndMain:
 Security:
   extensions:
     - ErrorPageControllerExtension
-HtmlEditorField:
-  extensions:
-    - SiteTreeLinkTracking_Highlighter

code/model/SiteTreeLinkTracking.php

@@ -37,15 +37,39 @@ class SiteTreeLinkTracking extends DataExtension {
 		"ImageTracking" => array("FieldName" => "Varchar")
 	);
 
-	function trackLinksInField($field) {
+	public function trackLinksInField($fieldName) {
 		$record = $this->owner;
 
 		$linkedPages = array();
 		$linkedFiles = array();
 
-		$htmlValue = Injector::inst()->create('HTMLValue', $record->$field);
+		$htmlValue = Injector::inst()->create('HTMLValue', $record->$fieldName);
 		$links = $this->parser->process($htmlValue);
 
+		// Highlight broken links in the content.
+		foreach ($links as $link) {
+			$classStr = trim($link['DOMReference']->getAttribute('class'));
+			if (!$classStr) {
+				$classes = array();
+			} else {
+				$classes = explode(' ', $classStr);
+			}
+
+			// Add or remove the broken class from the link, depending on the link status.
+			if ($link['Broken']) {
+				$classes = array_unique(array_merge($classes, array('ss-broken')));
+			} else {
+				$classes = array_diff($classes, array('ss-broken'));
+			}
+
+			if (!empty($classes)) {
+				$link['DOMReference']->setAttribute('class', implode(' ', $classes));
+			} else {
+				$link['DOMReference']->removeAttribute('class');
+			}
+		}
+		$record->$fieldName = $htmlValue->getContent();
+
 		// Populate link tracking for internal links & links to asset files.
 		foreach ($links as $link) {
 			switch ($link['Type']) {
@@ -88,13 +112,13 @@ class SiteTreeLinkTracking extends DataExtension {
 		if($record->ID && $record->many_many('LinkTracking') && $tracker = $record->LinkTracking()) {
 			$tracker->removeByFilter(sprintf(
 				'"FieldName" = \'%s\' AND "%s" = %d',
-				$field,
+				$fieldName,
 				$tracker->getForeignKey(),
 				$record->ID
 			));
 
 			if($linkedPages) foreach($linkedPages as $item) {
-				$tracker->add($item, array('FieldName' => $field));
+				$tracker->add($item, array('FieldName' => $fieldName));
 			}
 		}
 
@@ -102,18 +126,18 @@ class SiteTreeLinkTracking extends DataExtension {
 		if($record->ID && $record->many_many('ImageTracking') && $tracker = $record->ImageTracking()) {
 			$tracker->removeByFilter(sprintf(
 				'"FieldName" = \'%s\' AND "%s" = %d',
-				$field,
+				$fieldName,
 				$tracker->getForeignKey(),
 				$record->ID
 			));
 
 			if($linkedFiles) foreach($linkedFiles as $item) {
-				$tracker->add($item, array('FieldName' => $field));
+				$tracker->add($item, array('FieldName' => $fieldName));
 			}
 		}
 	}
 
-	function augmentSyncLinkTracking() {
+	public function augmentSyncLinkTracking() {
 		// Reset boolean broken flags
 		$this->owner->HasBrokenLink = false;
 		$this->owner->HasBrokenFile = false;
@@ -134,51 +158,6 @@ class SiteTreeLinkTracking extends DataExtension {
 	}
 }
 
-/**
- * Extension for enabling highlighting of broken links in the HtmlEditorFields.
- */
-class SiteTreeLinkTracking_Highlighter extends Extension {
-
-	public $parser;
-
-	private static $dependencies = array(
-		'parser' => '%$SiteTreeLinkTracking_Parser'
-	);
-
-	/**
-	 * Adds an ability to highlight broken links in the content.
-	 * It reuses the parser the SiteTreeLinkTracking uses for maintaining the references and the "broken" flags
-	 * to make sure all pages listed in the BrokenLinkChecker highlight these in their content.
-	 */
-	public function onBeforeRender($field) {
-		// Handle situation when the field has been customised, i.e. via $properties on the HtmlEditorField::Field call.
-		$obj = $this->owner->getCustomisedObj() ?: $this->owner;
-		$value = $obj->value;
-
-		// Parse the text as DOM.
-		$htmlValue = Injector::inst()->create('HTMLValue', $value);
-		$links = $this->parser->process($htmlValue);
-
-		foreach ($links as $link) {
-			$classStr = $link['DOMReference']->getAttribute('class');
-			$classes = explode(' ', $classStr);
-
-			// Add or remove the broken class from the link, depending on the link status.
-			if ($link['Broken']) {
-				$classes = array_unique(array_merge($classes, array('ss-broken')));
-			} else {
-				$classes = array_diff($classes, array('ss-broken'));
-			}
-			$link['DOMReference']->setAttribute('class', implode(' ', $classes));
-		}
-
-		$obj->customise(array(
-			'Value' => htmlentities($htmlValue->getContent(), ENT_COMPAT, 'UTF-8')
-		));
-	}
-
-}
-
 /**
  * A helper object for extracting information about links.
  */

tests/model/SiteTreeBrokenLinksTest.php

@@ -292,13 +292,6 @@ class SiteTreeBrokenLinksTest extends SapphireTest {
 		$this->assertFalse((bool)$vp->HasBrokenLink);
 		$this->assertFalse((bool)$rp->HasBrokenLink);
 
-		// However, the page isn't marked as modified on stage
-		$this->assertFalse($p2->IsModifiedOnStage);
-		$this->assertFalse($rp->IsModifiedOnStage);
-
-		// This is something that we know to be broken
-		//$this->assertFalse($vp->IsModifiedOnStage);
-
 	}
 
 	public function testBrokenAnchorLinksInAPage() {

tests/model/SiteTreeHTMLEditorFieldTest.php

@@ -166,37 +166,4 @@ class SiteTreeHtmlEditorFieldTest extends FunctionalTest {
 		$this->assertFalse((bool) $sitetree->HasBrokenFile);
 	}
 
-	public function testBrokenLinkHighlighting() {
-		$sitetree = new SiteTree();
-		$editor   = new HtmlEditorField('Content');
-
-		// SiteTree link highlighting
-		$editor->setValue('<a href="[sitetree_link,id=0]">Broken Link</a>');
-
-		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
-		$this->assertContains('ss-broken', (string) $element['class'], 'A broken SiteTree link is highlighted');
-
-		$editor->setValue(sprintf (
-			'<a href="[sitetree_link,id=%d]">Working Link</a>',
-			$this->idFromFixture('SiteTree', 'home')
-		));
-
-		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
-		$this->assertNotContains('ss-broken', (string) $element['class']);
-
-		// File link highlighting
-		$editor->setValue('<a href="[file_link,id=0]">Broken Link</a>');
-
-		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
-		$this->assertContains('ss-broken', (string) $element['class'], 'A broken File link is highlighted');
-
-		$editor->setValue(sprintf (
-			'<a href="[file_link,id=%d]">Working Link</a>',
-			$this->idFromFixture('File', 'example_file')
-		));
-
-		$element = new SimpleXMLElement(html_entity_decode((string) new SimpleXMLElement($editor->Field())));
-		$this->assertNotContains('ss-broken', (string) $element['class']);
-	}
-
 }

tests/model/SiteTreeLinkTrackingTest.php

@@ -35,10 +35,10 @@ class SiteTreeLinkTrackingTest extends SapphireTest {
 	}
 
 	function highlight($content) {
-		$field = new SiteTreeLinkTrackingTest_Field('Test');
-		$field->setValue($content);
-		$newContent = html_entity_decode($field->Field(), ENT_COMPAT, 'UTF-8');
-		return $newContent;
+		$page = new Page();
+		$page->Content = $content;
+		$page->write();
+		return $page->Content;
 	}
 
 	function testHighlighter() {
@@ -49,20 +49,15 @@ class SiteTreeLinkTrackingTest extends SapphireTest {
 		$content = $this->highlight('<a href="[sitetree_link,id=123]">link</a>');
 		$this->assertEquals(substr_count($content, 'ss-broken'), 1, 'ss-broken class is added to the broken link.');
 
-		$page = new Page();
-		$page->Content = '';
-		$page->write();
+		$otherPage = new Page();
+		$otherPage->Content = '';
+		$otherPage->write();
 
 		$content = $this->highlight(
-			"<a href=\"[sitetree_link,id=$page->ID]\" class=\"existing-class ss-broken ss-broken\">link</a>"
+			"<a href=\"[sitetree_link,id=$otherPage->ID]\" class=\"existing-class ss-broken ss-broken\">link</a>"
 		);
 		$this->assertEquals(substr_count($content, 'ss-broken'), 0, 'All ss-broken classes are removed from good link');
 		$this->assertEquals(substr_count($content, 'existing-class'), 1, 'Existing class is not removed.');
 	}
-}
 
-class SiteTreeLinkTrackingTest_Field extends HtmlEditorField implements TestOnly {
-	private static $extensions = array(
-		'SiteTreeLinkTracking_Highlighter'
-	);
 }

tests/tasks/MigrateSiteTreeLinkingTaskTest.php

@@ -29,12 +29,12 @@ class MigrateSiteTreeLinkingTaskTest extends SapphireTest {
 		$hashID   = $this->idFromFixture('SiteTree', 'hash_link');
 		
 		$homeContent = sprintf (
-			'<a href="[sitetree_link,id=%d]">About</a><a href="[sitetree_link,id=%d]">Staff</a><a href="http://silverstripe.org/">External Link</a>',
+			'<a href="[sitetree_link,id=%d]">About</a><a href="[sitetree_link,id=%d]">Staff</a><a href="http://silverstripe.org/">External Link</a><a name="anchor"></a>',
 			$aboutID,
 			$staffID
 		);
 		$aboutContent = sprintf (
-			'<a href="[sitetree_link,id=%d]">Home</a><a href="[sitetree_link,id=%d]">Staff</a>',
+			'<a href="[sitetree_link,id=%d]">Home</a><a href="[sitetree_link,id=%d]">Staff</a><a name="second-anchor"></a>',
 			$homeID,
 			$staffID
 		);

tests/tasks/MigrateSiteTreeLinkingTaskTest.yml

@@ -2,11 +2,11 @@ SiteTree:
   home:
     Title: Home Page
     URLSegment: home
-    Content: '<a href="about/">About</a><a href="staff">Staff</a><a href="http://silverstripe.org/">External Link</a>'
+    Content: '<a href="about/">About</a><a href="staff">Staff</a><a href="http://silverstripe.org/">External Link</a><a name="anchor"></a>'
   about:
     Title: About Us
     URLSegment: about
-    Content: '<a href="home">Home</a><a href="staff/">Staff</a>'
+    Content: '<a href="home">Home</a><a href="staff/">Staff</a><a name="second-anchor"></a>'
   staff:
     Title: Staff
     URLSegment: staff