When any of my tests fail because they are calling a function which is not defined I get an HTML-styled error message converted to plain text. See attached image for an example.
Either 1) stripping the tags or 2) not converting it to plain text will definitely make the message a bit more informative (and pretty).
Running Garland that comes with Drupal 6.4 as my theme.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 313902-screenshot.png | 68.61 KB | c960657 |
| #10 | 313902-2.patch | 1.48 KB | c960657 |
| #7 | 313902-1.patch | 6.36 KB | c960657 |
| #3 | test_crash.tar_.gz | 544 bytes | ztyx |
| beautify_error_message.jpg | 263.73 KB | ztyx |
Comments
Comment #1
boombatower commentedThis should be fixed in core and backported.
Comment #2
lilou commentedIt seems that this bug comes from your custom module "translation review.
How can we reproduce it ?
Comment #3
ztyx commentedOh, I thought the it was self explanatory how to reproduce it - just call a function that does not exist in one of your test cases. For simplicity I am attaching a sample module that contains a test that calls a function that does not exist. You should get the same output as in the earlier attached screenshot.
The module is called "test_crash".
Comment #4
ztyx commentedComment #5
drewish commentedyou shouldn't need a module for that, you should be able to trigger the error in a test case... just call NONEXISTANT_FUNCTION();
Comment #6
c960657 commentedI believe this problem is due to the
html_errorsini setting being enabled. The big HTML table is generated by XDebug, but even without XDebug you get HTML tags in the error message whenhtml_errorsis enabled.The PHP error message is generated in Drupal.ahahError() in misc/drupal.js where it is treated as plain text (it is inserted in the error string using
@textrather than!text). I don't think it is safe to include the PHP error using!text, but I am not completely aware of the threat model here.Comment #7
c960657 commentedThis patch disables
html_errorsfor all of Drupal. Would anybody miss those?Currently drupal_error_handler($errno, $message, ...) doesn't deal properly with HTML encoding. Whether
$messageis HTML or plain text depends onhtml_errors, but the function doesn't check that setting.$messageis passed directly to drupal_set_message($message, ...) that expects an HTML string, but in the watchdog() call it is inserted with%message, i.e. it is assumed to be plain text.Currently, when
html_errorsis off, error messages from tests are currently not being displayed. This is because of the lineif (jQuery.trim($(xmlhttp.responseText).text())) {.jQuery's
$(somestring)allows somestring to be either an HTML string or a selector. IfresponseTextdoesn't look like HTML, it is treated like a selector - and that does not match anything, and thus the output isn't displayed. This bug is also fixed by this patch.Comment #8
c960657 commentedReroll. The patch is now simpler due to the rewrite of the error handling system in #304924: Extend drupal_error_handler to manage exceptions.
Comment #9
damien tournoud commentedWe now have html_error set to 0 in drupal_initialize_variables. I'm not sure if the other bits are really needed (especially the check_plain() of the error message...).
Comment #10
c960657 commentedSorry, I forgot to attach the actual updated patch :-(
Comment #11
c960657 commentedComment #12
c960657 commentedUpdating title. With the disabling of
html_errorsno error messages are shown at all. I have attached an updated screenshot showing how errors are displayed with and without this patch.I don't know why the testbed robot could not install the patch. AFAICT it still applies to HEAD.
Comment #13
dries commentedCommitted to CVS HEAD. Thanks again. :-)
Comment #14
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.