Edit Handler
See also:
Documentation: EditHandlerInfoSince there are a number of issues with the edit handler in Wikka version 1.1.6.0 (one actually introduced with that version) I'm creating this development page to tackle them. --JavaWoman
Current Edit Handler
For reference, the code of the current (version 1.1.6.0) edit handler is as follows:
- <div class="page">
- <?php
- echo '<em>The page name is invalid. Valid page names must start with a letter and contain only letters and numbers.</em>';
- }
- elseif ($this->HasAccess("write") && $this->HasAccess("read"))
- {
- if ($newtag = $_POST['newtag']) $this->Redirect($this->Href('edit', $newtag));
- if ($_POST)
- {
- // strip CRLF line endings down to LF to achieve consistency ... plus it saves database space.
- // Note: these codes must remain enclosed in double-quotes to work! -- JsnX
- $body = preg_replace("/\n[ ]{4}/", "\n\t", $body); # @@@ FIXME: misses first line and multiple sets of four spaces - JW 2005-01-16
- // we don't need to escape here, we do that just before display (i.e., treat note just like body!)
- // only if saving:
- if ($_POST['submit'] == 'Store')
- {
- // check for overwriting
- if ($this->page)
- {
- if ($this->page['id'] != $_POST['previous'])
- {
- $error = 'OVERWRITE ALERT: This page was modified by someone else while you were editing it.<br />'."\n".'Please copy your changes and re-edit this page.';
- }
- }
- // store
- if (!$error)
- {
- // only save if new body differs from old body
- if ($body != $this->page['body']) {
- // add page (revisions)
- $this->SavePage($this->tag, $body, $note);
- // now we render it internally so we can write the updated link table.
- $this->ClearLinkTable();
- $this->StartLinkTracking();
- $dummy .= $this->Format($body);
- $dummy .= $this->Footer();
- $this->StopLinkTracking();
- $this->WriteLinkTable();
- $this->ClearLinkTable();
- }
- // forward
- $this->Redirect($this->Href());
- }
- }
- }
- // fetch fields
- if (!$previous = $_POST['previous']) $previous = $this->page['id'];
- if (!$body) $body = $this->page['body'];
- $body = preg_replace("/\n[ ]{4}/", "\n\t", $body); # @@@ FIXME: misses first line and multiple sets of four spaces - JW 2005-01-16
- }
- else
- {
- $maxtaglen = 75;
- }
- // preview?
- if ($_POST['submit'] == 'Preview') # preview page
- {
- $previewButtons =
- "<hr />\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // so we use htmlspecialchars on the edit note (as on the body)
- '<input size="50" type="text" name="note" value="'.htmlspecialchars($note).'"/> Note on your edit.<br />'."\n".
- '<input name="submit" type="submit" value="Store" accesskey="s" />'."\n".
- '<input name="submit" type="submit" value="Re-Edit" accesskey="p" />'."\n".
- '<input type="button" value="Cancel" onclick="document.location=\''.$this->href('').'\';" />'."\n";
- $output .= '<div class="previewhead">Preview</div>'."\n";
- $output .= $this->Format($body);
- $output .=
- $this->FormOpen('edit')."\n".
- '<input type="hidden" name="previous" value="'.$previous.'" />'."\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // hence htmlspecialchars() instead of htmlspecialchars_ent() which UNescapes entities!
- $output .=
- "<br />\n".
- $previewButtons.
- $this->FormClose()."\n";
- }
- {
- $this->tag = substr($this->tag, 0, $maxtaglen); // truncate tag to feed a backlinks-handler with the correct value. may be omited. it only works if the link to a backlinks-handler is built in the footer.
- $output = '<div class="error">Tag too long! $maxtaglen characters max.</div><br />'."\n";
- $output .= 'FYI: Clicking on Rename will automatically truncate the tag to the correct size.<br /><br />'."\n";
- $output .= $this->FormOpen('edit');
- $output .= '<input name="newtag" size="75" value="'.$this->htmlspecialchars_ent($this->tag).'" />';
- $output .= '<input name="submit" type="submit" value="Rename" />'."\n";
- $output .= $this->FormClose();
- }
- else # edit page
- {
- // display form
- if ($error)
- {
- $output .= '<div class="error">'.$error.'</div>'."\n";
- }
- // append a comment?
- if ($_REQUEST['appendcomment'])
- {
- }
- $output .=
- $this->FormOpen('edit').
- '<input type="hidden" name="previous" value="'.$previous.'" />'."\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // hence htmlspecialchars() instead of htmlspecialchars_ent() which UNescapes entities!
- '<textarea id="body" name="body" style="width: 100%; height: 500px">'.htmlspecialchars($body).'</textarea><br />'."\n".
- //note add Edit
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // so we use htmlspecialchars on the edit note (as on the body)
- '<input size="40" type="text" name="note" value="'.htmlspecialchars($note).'" /> Please add a note on your edit.<br />'."\n".
- //finish
- '<input name="submit" type="submit" value="Store" accesskey="s" /> <input name="submit" type="submit" value="Preview" accesskey="p" /> <input type="button" value="Cancel" onclick="document.location=\''.$this->Href('').'\';" />'."\n".
- $this->FormClose();
- if ($this->GetConfigValue('gui_editor') == 1) {
- $output .=
- '<script language="JavaScript" src="3rdparty/plugins/wikiedit/protoedit.js"></script>'."\n".
- '<script language="JavaScript" src="3rdparty/plugins/wikiedit/wikiedit2.js"></script>'."\n";
- $output .= '<script type="text/javascript">'." wE = new WikiEdit(); wE.init('body','WikiEdit','editornamecss');".'</script>'."\n";
- }
- }
- echo $output;
- }
- else
- {
- $message = '<em>You don\'t have write access to this page. You might need to register an account to get write access.</em><br />'."\n".
- "<br />\n".
- '<a href="'.$this->Href('showcode').'" title="Click to view page formatting code">View formatting code for this page</a>'.
- "<br />\n";
- echo $message;
- }
- ?>
- </div>
As TimoK stated on his user page, "I found the code quite hard to read". Clearly, it's not very well-structured, which makes it also hard to tackle the various problems with it.
Current issues
In no particular order, but numbered to make it easier to refer to them:
- Bad structure makes the code hard to understand and fix (see TimoK)
- The function of some of the code is unclear - possibly it is never used and could (should) be eliminated
- Various statements lead to notices because of reference to undefined variables
- Indents consisting of (4) spaces are translated into tabs in only a limited number of cases (only the first group of 4 spaces at the start of a line, and not on the first line of the page (see Indenting not working properly in handlers/page/edit.php on WikkaBugs)
- The code generated is not actually valid XHTML; in particular the <script> elements are not valid.
- Unicode characters that were entered as Unicode characters (rather than as character or numeric entity references) are rendered as numeric entity references in the textarea when editing the page, making the text very hard to edit since it becomes essentially unreadable (this bug was actually introduced in version 1.1.6.0; see 1.1.6.0beta4: Simplified Chinese (or Unicode relative) in WikiEdit: on WikkaBugs)
- There is also an issue with Unicode characters in code blocks (such as in comments!) being converted to numeric entity references but it's unclear for now whether this is caused by the edit handler or by the GeSHi code highlighting. (An example of this is (still) found in the SandBox - see also Missing language-support in the code formatter on WikkaBugs)
- For some sites it's desirable to have the edit note be a required field; this is currently not supported (see Note on edit, mandatory field on SuggestionBox)
- While registered users do have an option to set whether or not to use double-click editing, they cannot set a preference for using the WikiEdit toolbar. Since some people don't actually use the toolbar while this slows down editing, this really should be an option.
- There is no way to indicate whether a page edit is "minor" (for instance, correcting a typo or a small change to layout without effectively changing the page content); minor changes should not be displayed in RecentChanges, and not broadcast in the RecentChanges RSS feed or WikiPing. Especially RSS feeds and WikiPing will be more valuable if they are not cluttered up with with irrelevant changes.
Tackling the issues
Issue 3: Notices
On Alan (my development system) I'm running with error_reporting(E_ALL); so all errors, warnings and notices get reported - instead of the (Wikka) default of error_reporting (E_ALL ^ E_NOTICE); which suppresses notices. This is to catch any notices (mostly caused by using uninitialized variables which can lead to hard-to-track bugs) and squash them where I find them.Since the edit handler invariably caused a number of notices, this is the first thing I tackled. The fix for such notices is always easy: make sure a variable is actually assigned an (initial) value before trying to evaluate it. (Ideally Wikka should be completely notice-free but we can get there by squashing them where we find them.)
Issue 4: Indenting not working properly
The code that attempts to change (leading) groups of four space into tabs actually occurs twice in the code (see issue 1!) on lines 15 and 60: $body = preg_replace("/\n[ ]{4}/", "\n\t", $body); # @@@ FIXME: misses first line and multiple sets of four spaces - JW 2005-01-16
As indicated on WikkaBugs, I came up with the following solution:
# JW FIXED 2005-07-12
$pattern = '/^(\t*) {4}/m'; # m modifier: match ^ at start of line *and* at start of string;
$replace = "$1\t";
while (preg_match($pattern,$body))
{
$body = preg_replace($pattern,$replace,$body);
}
# @@@ NOTE: This could be easily extended to also change a '~' into a tab. But should we? '~' is easy to type and edit
$pattern = '/^(\t*) {4}/m'; # m modifier: match ^ at start of line *and* at start of string;
$replace = "$1\t";
while (preg_match($pattern,$body))
{
$body = preg_replace($pattern,$replace,$body);
}
# @@@ NOTE: This could be easily extended to also change a '~' into a tab. But should we? '~' is easy to type and edit
Issue 5: Invalid XHTML generated
Simply solved by ensuring that all <script> elements have the required type="text/javascript" attribute.
More later...
The code with the solutions for issues 3, 4 and 5 is now implemented as WikkaBetaFeatures beta on this site. Other issues to be tackled later.
This beta code also contains an experimental anti-spam feature: link throttling (to be documented later). For obvious reasons the exact number used in the link throttling is not revealed here; besides, it should become a configuration option rather than be hard-coded as it is for now.
The current beta code now looks as follows (replace XXX with whatever number you wish for link throttling):
- <div class="page">
- <?php
- // various minor changes to prevent NOTICES (due to uninitialized variables) - JW 2005-06-07
- // added type="text/javascript" to script elements (valid XHTML) - JW 2005-07-01
- echo '<em>The page name is invalid. Valid page names must start with a letter and contain only letters and numbers.</em>';
- }
- elseif ($this->HasAccess("write") && $this->HasAccess("read"))
- {
- #if ($newtag = $_POST['newtag']) $this->Redirect($this->Href('edit', $newtag));
- {
- $newtag = $_POST['newtag'];
- $this->Redirect($this->Href('edit', $newtag));
- }
- // init fields
- $body = '';
- $note = '';
- $submit = '';
- $output = '';
- if ($_POST)
- {
- {
- // strip CRLF line endings down to LF to achieve consistency ... plus it saves database space.
- // Note: these codes must remain enclosed in double-quotes to work! -- JsnX
- // replace each 4 consecutive spaces at the start of a line with a tab
- #$body = preg_replace("/\n[ ]{4}/", "\n\t", $body); # @@@ FIXME: misses first line and multiple sets of four spaces - JW 2005-01-16
- # JW FIXED 2005-07-12
- $pattern = '/^(\t*) {4}/m'; # m modifier: match ^ at start of line *and* at start of string;
- $replace = "$1\t";
- {
- }
- # @@@ NOTE: This could be easily extended to also change a '~' into a tab. But should we? '~' is easy to type and edit
- }
- // we don't need to escape here, we do that just before display (i.e., treat note just like body!)
- {
- }
- {
- $submit = $_POST['submit'];
- }
- // only if saving:
- if ($submit == 'Store')
- {
- // check for overwriting
- if ($this->page)
- {
- if ($this->page['id'] != $_POST['previous'])
- {
- $error = 'OVERWRITE ALERT: This page was modified by someone else while you were editing it.<br />'."\n".'Please copy your changes and re-edit this page.';
- }
- // check for spam - antispam measure added by JsnX 2005-03-21; modified 2005-03-25 (BETA)
- if ($new_link_count >= $existing_link_count + XXX) # @@@ replace XXX by your own number!
- {
- $error = 'SPAM ALERT: It appears that you are trying to spam.<br />'."\n".$this->Format('If you are not spamming, please report the situation on Wikka:WikkaBugs.');
- }
- // end anti-spam BETA
- }
- // store
- {
- // only save if new body differs from old body
- if ($body != $this->page['body']) {
- // add page (revisions)
- $this->SavePage($this->tag, $body, $note);
- // now we render it internally so we can write the updated link table.
- $this->ClearLinkTable();
- $this->StartLinkTracking();
- $dummy .= $this->Format($body);
- $dummy .= $this->Footer();
- $this->StopLinkTracking();
- $this->WriteLinkTable();
- $this->ClearLinkTable();
- }
- // forward
- $this->Redirect($this->Href());
- }
- }
- }
- // fetch fields
- #if (!$previous = $_POST['previous']) $previous = $this->page['id'];
- if (!$body) $body = $this->page['body']; # @@@ ??
- // replace each 4 consecutive spaces at the start of a line with a tab
- #$body = preg_replace("/\n[ ]{4}/", "\n\t", $body); # @@@ FIXME: misses first line and multiple sets of four spaces - JW 2005-01-16
- # JW FIXED 2005-07-12
- $pattern = '/^(\t*) {4}/m'; # m modifier: match ^ at start of line *and* at start of string;
- $replace = "$1\t";
- {
- }
- // derive maximum length for a page name from the table structure
- {
- }
- else
- {
- $maxtaglen = 75;
- }
- // preview?
- if ($submit == 'Preview') # preview page
- {
- $previewButtons =
- "<hr />\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // so we use htmlspecialchars on the edit note (as on the body)
- '<input size="50" type="text" name="note" value="'.htmlspecialchars($note).'"/> Note on your edit.<br />'."\n".
- '<input name="submit" type="submit" value="Store" accesskey="s" />'."\n".
- '<input name="submit" type="submit" value="Re-Edit" accesskey="p" />'."\n".
- '<input type="button" value="Cancel" onclick="document.location=\''.$this->href('').'\';" />'."\n";
- $output .= '<div class="previewhead">Preview</div>'."\n";
- $output .= $this->Format($body);
- $output .=
- $this->FormOpen('edit')."\n".
- '<input type="hidden" name="previous" value="'.$previous.'" />'."\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // hence htmlspecialchars() instead of htmlspecialchars_ent() which UNescapes entities!
- $output .=
- "<br />\n".
- $previewButtons.
- $this->FormClose()."\n";
- }
- {
- // truncate tag to feed a backlinks-handler with the correct value. may be omited. it only works if the link to a backlinks-handler is built in the footer.
- // @@@ backlinks handler ???
- $output = '<div class="error">Tag too long! '.$maxtaglen.' characters max.</div><br />'."\n";
- $output .= 'FYI: Clicking on Rename will automatically truncate the tag to the correct size.<br /><br />'."\n";
- $output .= $this->FormOpen('edit');
- $output .= '<input name="newtag" size="75" value="'.$this->htmlspecialchars_ent($this->tag).'" />';
- $output .= '<input name="submit" type="submit" value="Rename" />'."\n";
- $output .= $this->FormClose();
- }
- else # edit page
- {
- // display form
- {
- $output .= '<div class="error">'.$error.'</div>'."\n";
- }
- // append a comment?
- if ($_REQUEST['appendcomment'])
- {
- }
- $output .=
- $this->FormOpen('edit').
- '<input type="hidden" name="previous" value="'.$previous.'" />'."\n".
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // hence htmlspecialchars() instead of htmlspecialchars_ent() which UNescapes entities!
- '<textarea id="body" name="body" style="width: 100%; height: 500px">'.htmlspecialchars($body).'</textarea><br />'."\n".
- //note add Edit
- // We need to escape ALL entity refs before display so we display them _as_ entities instead of interpreting them
- // so we use htmlspecialchars on the edit note (as on the body)
- '<input size="40" type="text" name="note" value="'.htmlspecialchars($note).'" /> Please add a note on your edit.<br />'."\n".
- //finish
- '<input name="submit" type="submit" value="Store" accesskey="s" /> <input name="submit" type="submit" value="Preview" accesskey="p" /> <input type="button" value="Cancel" onclick="document.location=\''.$this->Href('').'\';" />'."\n".
- $this->FormClose();
- if ($this->GetConfigValue('gui_editor') == 1) {
- $output .=
- '<script language="JavaScript" type="text/javascript" src="3rdparty/plugins/wikiedit/protoedit.js"></script>'."\n".
- '<script language="JavaScript" type="text/javascript" src="3rdparty/plugins/wikiedit/wikiedit2.js"></script>'."\n".
- '<script language="JavaScript" type="text/javascript">'." wE = new WikiEdit(); wE.init('body','WikiEdit','editornamecss');".'</script>'."\n";
- }
- }
- echo $output;
- }
- else
- {
- $message = '<em>You don\'t have write access to this page. You might need to register an account to get write access.</em><br />'."\n".
- "<br />\n".
- '<a href="'.$this->Href('showcode').'" title="Click to view page formatting code">View formatting code for this page</a>'.
- "<br />\n";
- echo $message;
- }
- ?>
- </div>
CategoryDevelopmentHandlers