From bb1e78082c9a3d5a74e13384ddaa9683e2210340 Mon Sep 17 00:00:00 2001 From: William Jacoby Date: Wed, 14 Oct 2026 12:08:49 -0500 Subject: [PATCH] Add a readable view to the version compare The compare page opens in a Readable view: both versions rendered, split into lines, headings, list items and table rows, and compared with phpBB's diff engine. Unchanged parts keep their formatting, added and removed parts are marked whole, a part whose words changed shows which ones, and a part whose formatting alone changed is outlined. Parts with an image are shown whole. Source keeps the BBCode diff; Template: pages use it only. Also: comparing now needs u_wiki_versions, like the version history, and both versions must belong to the article in the address; the description diff no longer hides behind the sources' diff; and NO_VERSIONS_SELECTED has its text. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- docs/TODO.md | 6 - language/en/common.php | 4 + .../prosilver/template/article_compare.html | 10 +- styles/prosilver/theme/style.css | 22 + tests/unit/readable_diff_test.php | 82 ++++ wiki/compare.php | 62 ++- wiki/readable_diff.php | 456 ++++++++++++++++++ 8 files changed, 631 insertions(+), 13 deletions(-) create mode 100644 tests/unit/readable_diff_test.php create mode 100644 wiki/readable_diff.php diff --git a/README.md b/README.md index 7c9c877..869a873 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A wiki for phpBB with version history, side-by-side compare, and a moderation qu - A wiki at `/wiki`, with an overview page listing its articles. - Start an article from the overview page's **New article title** box, or by linking to `/wiki/` and following the link. - Link to another article by writing `[[Article name]]`, in articles and in forum posts; links to articles that don't exist yet are shown in red and open the editor. Each article lists the articles that link to it under **What links here**. See [Linking between articles](#linking-between-articles). -- Every edit is saved as a new version; compare any two versions side by side. +- Every edit is saved as a new version; compare any two versions. The compare page opens in a **Readable** view: the newer version as readers see it, with added text in green and underlined, removed text in red and struck through, and the changed words marked inside a line, list item or table row that changed. **Source** shows the BBCode diff. A `Template:` page is compared as written. - **Restore** in the version history brings back an older version by saving its text, title, description, sources and redirect as a new version, so no version is lost. Members who may edit can restore; the restored version goes live straight away for members who may set versions active, and otherwise waits for approval like any edit. - Edits can wait for approval: moderators get a **Pending articles** list with Approve and Disapprove. Approving a version older than the live one is blocked, so a stale draft can't overwrite newer content. - Moderators can take an article offline or delete it, and articles can be made sticky or set to redirect. diff --git a/docs/TODO.md b/docs/TODO.md index d26d890..6203a7d 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -131,9 +131,3 @@ header check, and holds no templates yet. Still to build: the templates themselv button. Admins upload its zip in the ACP or copy the files into a folder in `store/`. Plan, licence rules and decisions: [`docs/TODO/todo-wiki-templates-repo.md`](TODO/todo-wiki-templates-repo.md). - -## Readable version compare - -The compare page shows raw BBCode with changes marked in red and green. -Comparing the rendered text would be easier for editors who don't read -BBCode comfortably. diff --git a/language/en/common.php b/language/en/common.php index aeb074c..3ba97aa 100644 --- a/language/en/common.php +++ b/language/en/common.php @@ -58,6 +58,10 @@ 'VERSIONS_OF_ARTICLE' => 'Version history', 'VERSION_COMPARE_HEADLINE' => 'Changes from version %1$d to %2$d', 'COMPARE' => 'Compare', + 'NO_VERSIONS_SELECTED' => 'Choose two versions of this article to compare.', + 'COMPARE_VIEW_READABLE' => 'Readable', + 'COMPARE_VIEW_SOURCE' => 'Source', + 'COMPARE_READABLE_EXPLAIN' => 'The newer version as readers see it. Added text is green and underlined, removed text is red and struck through, and a part whose words changed shows which ones. A part outlined in blue changed only its formatting; the source view shows how.', 'COMPARE_EXPLAIN' => 'All versions of the article are listed here. Choose two versions to compare them.', 'VERSION' => 'Version', 'TITLE' => 'Title', diff --git a/styles/prosilver/template/article_compare.html b/styles/prosilver/template/article_compare.html index fbdcbb4..5a6d861 100644 --- a/styles/prosilver/template/article_compare.html +++ b/styles/prosilver/template/article_compare.html @@ -10,9 +10,17 @@

{L_ARTICLE_DESCRIPTION}

+ +

+ {L_COMPARE_VIEW_READABLE}{L_COMPARE_VIEW_READABLE} + • + {L_COMPARE_VIEW_SOURCE}{L_COMPARE_VIEW_SOURCE} +
{L_COMPARE_READABLE_EXPLAIN} +

+
-
+
{DIFF} diff --git a/styles/prosilver/theme/style.css b/styles/prosilver/theme/style.css index 6f93d0e..9b188c7 100644 --- a/styles/prosilver/theme/style.css +++ b/styles/prosilver/theme/style.css @@ -174,3 +174,25 @@ form.wiki-start-from p { padding: 0.5em 0; overflow-x: auto; } + +/* The readable compare: added, removed and reformatted parts of the newer version */ +.wiki-readable-diff ins.wiki-diff-ins, +.wiki-readable-diff .wiki-diff-ins { + background-color: #d4f8d4; + text-decoration: underline; +} +.wiki-readable-diff del.wiki-diff-del, +.wiki-readable-diff .wiki-diff-del { + background-color: #ffd7d5; + text-decoration: line-through; +} +.wiki-readable-diff .wiki-diff-changed { + outline: 1px dashed #4c5d77; + outline-offset: 2px; +} +.wiki-readable-diff .wiki-diff-line { + min-height: 1.4em; +} +.wiki-compare-explain { + font-size: 0.9em; +} diff --git a/tests/unit/readable_diff_test.php b/tests/unit/readable_diff_test.php new file mode 100644 index 0000000..2229697 --- /dev/null +++ b/tests/unit/readable_diff_test.php @@ -0,0 +1,82 @@ +Second line', 'Hello wide world
Second line'); + + $this->assertSame('
Hello wide world
Second line
', $html); + } + + public function test_an_added_heading_is_marked_whole_and_the_rest_is_unchanged() + { + $html = readable_diff::render('Intro
End', 'Intro

New

End'); + + $this->assertSame('
Intro

New

End
', $html); + } + + public function test_a_list_is_compared_item_by_item() + { + $html = readable_diff::render('
  • one
  • two
', '
  • one
  • two!
  • three
'); + + $this->assertSame('
  • one
  • two two!
  • three
', $html); + } + + public function test_a_table_row_is_compared_cell_by_cell() + { + $html = readable_diff::render( + '
A1
B2
', + '
A1
B3
' + ); + + $this->assertSame('
A1
B2 3
', $html); + } + + public function test_a_change_of_formatting_alone_is_marked_as_changed() + { + $this->assertSame('
plain text
', readable_diff::render('plain text', 'plain text')); + } + + public function test_a_line_with_an_image_is_shown_whole_not_as_words() + { + $html = readable_diff::render('See Getting Started.
End', 'logo
End'); + + $this->assertSame('
logo
End
', $html); + } + + public function test_text_is_escaped_and_unicode_kept() + { + $this->assertSame('
Grüße <aus> Köln Berlin
', readable_diff::render('Grüße <aus> Köln', 'Grüße <aus> Berlin')); + } + + public function test_removing_everything_and_comparing_nothing() + { + $this->assertSame('
gone
', readable_diff::render('gone', '')); + $this->assertSame('', readable_diff::render('', '')); + } +} diff --git a/wiki/compare.php b/wiki/compare.php index b57443b..ff9a270 100644 --- a/wiki/compare.php +++ b/wiki/compare.php @@ -46,6 +46,30 @@ class compare /** @var string php_ext */ protected $php_ext; + /** + * A version's text rendered as the article page shows it. + * + * @param string $article The article's name, as in its address + * @param array $row The version, with article_text, bbcode_uid and bbcode_bitfield + * @return string HTML + */ + protected function rendered($article, array $row) + { + // [[File:name]] and the PAGENAME words refer to this article; categories found + // while rendering are not this page's to list + \phpbbmodders\wiki\wiki\files::set_context($article); + \phpbbmodders\wiki\wiki\categories::reset(); + try + { + return generate_text_for_display($row['article_text'], $row['bbcode_uid'], $row['bbcode_bitfield'], OPTION_FLAG_BBCODE | OPTION_FLAG_SMILIES | OPTION_FLAG_LINKS); + } + finally + { + \phpbbmodders\wiki\wiki\files::set_context(null); + \phpbbmodders\wiki\wiki\categories::reset(); + } + } + /** * Constructor * @@ -78,6 +102,12 @@ public function __construct(\phpbb\auth\auth $auth, \phpbb\config\config $config public function compare_versions($article, $from, $to) { + // Comparing shows the text of versions, as the version history does + if (!$this->auth->acl_get('u_wiki_versions')) + { + trigger_error('NOT_AUTHORISED'); + } + if ($from == 0 || $to == 0) { trigger_error('NO_VERSIONS_SELECTED'); @@ -87,17 +117,25 @@ public function compare_versions($article, $from, $to) require($this->phpbb_root_path . 'includes/diff/engine.' . $this->php_ext); require($this->phpbb_root_path . 'includes/diff/renderer.' . $this->php_ext); - $sql = 'SELECT article_text, bbcode_uid, bbcode_bitfield, article_sources, article_description, article_approved + $sql = 'SELECT article_url, article_text, bbcode_uid, bbcode_bitfield, article_sources, article_description, article_approved FROM ' . $this->article_table . ' WHERE article_id = ' . (int) $from; $result = $this->db->sql_query($sql); $from_row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); - $sql = 'SELECT article_text, bbcode_uid, bbcode_bitfield, article_sources, article_description, article_approved + $sql = 'SELECT article_url, article_text, bbcode_uid, bbcode_bitfield, article_sources, article_description, article_approved FROM ' . $this->article_table . ' WHERE article_id = ' . (int) $to; $result = $this->db->sql_query($sql); $to_row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + // Both versions must exist and belong to the article in the address + if (!$from_row || !$to_row || $from_row['article_url'] !== $article || $to_row['article_url'] !== $article) + { + trigger_error('NO_VERSIONS_SELECTED'); + } $from_article = generate_text_for_edit($from_row['article_text'], $from_row['bbcode_uid'], OPTION_FLAG_BBCODE | OPTION_FLAG_SMILIES | OPTION_FLAG_LINKS); $to_article = generate_text_for_edit($to_row['article_text'], $to_row['bbcode_uid'], OPTION_FLAG_BBCODE | OPTION_FLAG_SMILIES | OPTION_FLAG_LINKS); @@ -111,18 +149,32 @@ public function compare_versions($article, $from, $to) $sources_diff_empty = $sources_diff->is_empty(); $description_diff = new \diff($from_row['article_description'], $to_row['article_description']); - $descriptiondiff_empty = $sources_diff->is_empty(); + $description_diff_empty = $description_diff->is_empty(); $renderer = new \diff_renderer_inline(); + // The readable view compares the versions as readers see them. A template page's + // text does not make sense rendered on its own, so it is compared as written. + $s_readable_possible = (strpos($article, \phpbbmodders\wiki\wiki\templates::PREFIX) !== 0); + $s_readable = $s_readable_possible && ($this->request->variable('view', 'readable') !== 'source'); + $readable = ''; + if ($s_readable && !$article_diff_empty) + { + $readable = \phpbbmodders\wiki\wiki\readable_diff::render($this->rendered($article, $from_row), $this->rendered($article, $to_row)); + } + $s_set_active = $this->auth->acl_get('u_wiki_set_active'); $s_delete = $this->auth->acl_get('m_wiki_delete'); $this->template->assign_vars(array( 'HEADLINE' => sprintf($this->user->lang('VERSION_COMPARE_HEADLINE'), $from, $to, $u_from, $u_to), - 'DIFF' => ($article_diff_empty) ? '' : $renderer->get_diff_content($article_diff), + 'DIFF' => ($article_diff_empty) ? '' : ($s_readable ? $readable : $renderer->get_diff_content($article_diff)), 'DIFF_SOURCE' => ($sources_diff_empty) ? '' : $renderer->get_diff_content($sources_diff), - 'DIFF_DESCRIPTION' => ($descriptiondiff_empty) ? '' : $renderer->get_diff_content($description_diff), + 'DIFF_DESCRIPTION' => ($description_diff_empty) ? '' : $renderer->get_diff_content($description_diff), + 'S_READABLE' => $s_readable, + 'S_READABLE_POSSIBLE' => $s_readable_possible, + 'U_VIEW_READABLE' => $this->helper->route('phpbbmodders_wiki_article', array('article' => $article, 'action' => 'compare', 'from' => (int) $from, 'to' => (int) $to)), + 'U_VIEW_SOURCE' => $this->helper->route('phpbbmodders_wiki_article', array('article' => $article, 'action' => 'compare', 'from' => (int) $from, 'to' => (int) $to, 'view' => 'source')), 'APPROVE_VERSION_FROM' => sprintf($this->user->lang('APPROVE_VERSION'), $from), 'REJECT_VERSION_FROM' => sprintf($this->user->lang('REJECT_VERSION'), $from), diff --git a/wiki/readable_diff.php b/wiki/readable_diff.php new file mode 100644 index 0000000..d861bcf --- /dev/null +++ b/wiki/readable_diff.php @@ -0,0 +1,456 @@ +compare(self::blocks(self::load($from_html)), self::blocks(self::load($to_html))); + } + + /** + * Parse an HTML fragment. + * + * @param string $html + * @return \DOMElement The element holding the fragment + */ + protected static function load($html) + { + $doc = new \DOMDocument(); + $errors = libxml_use_internal_errors(true); + $doc->loadHTML('
' . $html . '
', LIBXML_HTML_NODEFDTD | LIBXML_HTML_NOIMPLIED); + libxml_clear_errors(); + libxml_use_internal_errors($errors); + + return $doc->getElementById('wiki-diff-root') ?: $doc->documentElement; + } + + /** + * Split a fragment into blocks: block elements stay whole, and running text is cut + * at its line breaks, each line becoming a block of its own. Empty lines are dropped. + * + * @param \DOMNode $root + * @return array DOMNode blocks; a line of running text is an array of its nodes + */ + protected static function blocks(\DOMNode $root) + { + $blocks = array(); + $line = array(); + $flush = function () use (&$blocks, &$line) { + if (trim(self::text_of($line)) !== '' || self::has_media($line)) + { + $blocks[] = $line; + } + $line = array(); + }; + + foreach (iterator_to_array($root->childNodes) as $node) + { + $name = strtolower($node->nodeName); + if ($name === 'br') + { + $flush(); + } + else if ($node instanceof \DOMElement && in_array($name, self::BLOCKS, true)) + { + $flush(); + $blocks[] = $node; + } + else if (!($node instanceof \DOMComment)) + { + $line[] = $node; + } + } + $flush(); + + return $blocks; + } + + /** + * Compare two lists of blocks. + * + * @param array $from + * @param array $to + * @return string + */ + protected function compare(array $from, array $to) + { + $html = ''; + $i = $j = 0; + foreach ($this->edits($from, $to) as $edit) + { + $orig = array_slice($from, $i, $edit->norig()); + $final = array_slice($to, $j, $edit->nfinal()); + $i += $edit->norig(); + $j += $edit->nfinal(); + + if ($edit instanceof \diff_op_copy) + { + foreach ($final as $block) + { + $html .= $this->block_html($block, ''); + } + } + else + { + $html .= $this->changed($orig, $final); + } + } + + return $html; + } + + /** + * The edits that turn one list of blocks into the other, worked out on a key per block. + * + * @param array $from + * @param array $to + * @return array diff_op objects + */ + protected function edits(array $from, array $to) + { + $from_keys = array_map(array(__CLASS__, 'key'), $from); + $to_keys = array_map(array(__CLASS__, 'key'), $to); + if (!$from_keys && !$to_keys) + { + return array(); + } + if (!$from_keys) + { + return array(new \diff_op_add($to_keys)); + } + if (!$to_keys) + { + return array(new \diff_op_delete($from_keys)); + } + + $engine = new \diff_engine(); + + return $engine->diff($from_keys, $to_keys); + } + + /** + * Blocks that were replaced: pairs of the same kind are compared inside, anything + * left over is shown as removed or added. + * + * @param array $orig + * @param array $final + * @return string + */ + protected function changed(array $orig, array $final) + { + $html = ''; + $pairs = min(count($orig), count($final)); + for ($k = 0; $k < $pairs; $k++) + { + $html .= $this->changed_pair($orig[$k], $final[$k]); + } + for ($k = $pairs; $k < count($orig); $k++) + { + $html .= $this->block_html($orig[$k], 'del'); + } + for ($k = $pairs; $k < count($final); $k++) + { + $html .= $this->block_html($final[$k], 'ins'); + } + + return $html; + } + + /** + * One block that became another. + * + * @param \DOMNode|array $old + * @param \DOMNode|array $new + * @return string + */ + protected function changed_pair($old, $new) + { + $old_name = self::name($old); + $new_name = self::name($new); + + // A list or table that changed: compare its items or rows + if ($old_name === $new_name && in_array($new_name, self::CONTAINERS, true)) + { + return $this->open_tag($new) . $this->compare(self::children($old), self::children($new)) . ''; + } + + // Only the formatting changed: show the new block, marked as changed + if ($old_name === $new_name && self::text_of($old) === self::text_of($new)) + { + return $this->block_html($new, 'changed'); + } + + // A line, heading, list item or table row whose words changed. A word diff shows + // text only, so a part with an image in it is shown whole instead. + $has_media = self::has_media(is_array($old) ? $old : array($old)) || self::has_media(is_array($new) ? $new : array($new)); + if (!$has_media && $old_name === $new_name) + { + $words = self::word_diff(self::text_of($old), self::text_of($new)); + if (is_array($new)) + { + return '
' . $words . '
'; + } + if ($new_name === 'tr') + { + return $this->changed_row($old, $new, $words); + } + + return $this->open_tag($new) . $words . ''; + } + + return $this->block_html($old, 'del') . $this->block_html($new, 'ins'); + } + + /** + * A table row whose text changed: compared cell by cell when it kept its cells, + * otherwise shown as one cell across the row. + * + * @param \DOMElement $old + * @param \DOMElement $new + * @param string $words The word diff of the whole row + * @return string + */ + protected function changed_row(\DOMElement $old, \DOMElement $new, $words) + { + $old_cells = self::children($old); + $new_cells = self::children($new); + if (count($old_cells) !== count($new_cells)) + { + return $this->open_tag($new) . '' . $words . ''; + } + + $html = $this->open_tag($new); + foreach ($new_cells as $k => $cell) + { + $html .= (self::key($old_cells[$k]) === self::key($cell)) + ? $this->block_html($cell, '') + : $this->open_tag($cell) . self::word_diff(self::text_of($old_cells[$k]), self::text_of($cell)) . 'nodeName) . '>'; + } + + return $html . ''; + } + + /** + * The words of two texts with removed ones in and added ones in . + * + * @param string $old + * @param string $new + * @return string + */ + protected static function word_diff($old, $new) + { + $old_words = preg_split('/\s+/u', trim($old), -1, PREG_SPLIT_NO_EMPTY); + $new_words = preg_split('/\s+/u', trim($new), -1, PREG_SPLIT_NO_EMPTY); + if (!$old_words || !$new_words) + { + $edits = $old_words ? array(new \diff_op_delete($old_words)) : array(new \diff_op_add($new_words)); + } + else + { + $engine = new \diff_engine(); + $edits = $engine->diff($old_words, $new_words); + } + + $parts = array(); + $escape = function ($words) { + return htmlspecialchars(implode(' ', $words), ENT_COMPAT, 'UTF-8'); + }; + foreach ($edits as $edit) + { + if ($edit instanceof \diff_op_copy) + { + $parts[] = $escape($edit->final); + continue; + } + if ($edit->orig) + { + $parts[] = '' . $escape($edit->orig) . ''; + } + if ($edit->final) + { + $parts[] = '' . $escape($edit->final) . ''; + } + } + + return implode(' ', $parts); + } + + /** + * HTML of a block, marked as removed ("del"), added ("ins"), changed, or not at all (""). + * + * @param \DOMNode|array $block + * @param string $mark + * @return string + */ + protected function block_html($block, $mark) + { + $class = ($mark === '') ? '' : 'wiki-diff-' . $mark; + + if (is_array($block)) + { + $inner = ''; + foreach ($block as $node) + { + $inner .= $node->ownerDocument->saveHTML($node); + } + + return '
' . $inner . '
'; + } + + if ($class && $block instanceof \DOMElement) + { + $block = $block->cloneNode(true); + $block->setAttribute('class', trim($block->getAttribute('class') . ' ' . $class)); + } + + return $block->ownerDocument->saveHTML($block); + } + + /** + * The opening tag of an element, with its attributes. + * + * @param \DOMElement $element + * @return string + */ + protected function open_tag(\DOMElement $element) + { + $tag = '<' . strtolower($element->nodeName); + foreach ($element->attributes as $attribute) + { + $tag .= ' ' . $attribute->nodeName . '="' . htmlspecialchars($attribute->nodeValue, ENT_COMPAT, 'UTF-8') . '"'; + } + + return $tag . '>'; + } + + /** + * What two blocks are compared by: their kind and their text. + * + * @param \DOMNode|array $block + * @return string + */ + protected static function key($block) + { + $html = ''; + foreach (is_array($block) ? $block : array($block) as $node) + { + $html .= $node->ownerDocument->saveHTML($node); + } + + // The markup counts too, so a change of formatting alone is not taken for no change + return self::name($block) . ':' . self::text_of($block) . ':' . md5($html); + } + + /** + * @param \DOMNode|array $block + * @return string The element name, "line" for a line of running text + */ + protected static function name($block) + { + return is_array($block) ? 'line' : strtolower($block->nodeName); + } + + /** + * The text of a block on one line, with runs of white space made one space. + * + * @param \DOMNode|array $block + * @return string + */ + protected static function text_of($block) + { + $text = ''; + foreach (is_array($block) ? $block : array($block) as $node) + { + $text .= $node->textContent; + } + + return trim(preg_replace('/\s+/u', ' ', $text)); + } + + /** + * @param array $nodes + * @return bool Whether the nodes hold an image or another embedded item + */ + protected static function has_media(array $nodes) + { + foreach ($nodes as $node) + { + if ($node instanceof \DOMElement && (in_array(strtolower($node->nodeName), array('img', 'iframe', 'video', 'audio'), true) || $node->getElementsByTagName('img')->length)) + { + return true; + } + } + + return false; + } + + /** + * The items of a list or the rows of a table. + * + * @param \DOMElement $element + * @return array + */ + protected static function children(\DOMElement $element) + { + $children = array(); + foreach ($element->childNodes as $child) + { + if ($child instanceof \DOMElement) + { + $children[] = $child; + } + } + + return $children; + } + + /** + * @param \DOMElement $row + * @return int The number of columns a table row spans + */ + protected static function cell_count(\DOMElement $row) + { + $count = 0; + foreach (self::children($row) as $cell) + { + $count += max(1, (int) $cell->getAttribute('colspan')); + } + + return $count; + } +}