Last Comment Bug 321169 - XUL templates need better logging capability
: XUL templates need better logging capability
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 Linux
: -- normal with 3 votes (vote)
: mozilla1.9alpha8
Assigned To: Neil Deakin
:
Mentors:
Depends on: 285631 321171
Blocks: 330010 330012 543710
  Show dependency treegraph
 
Reported: 2005-12-21 20:15 PST by Neil Deakin
Modified: 2011-01-12 11:35 PST (History)
9 users (show)
enndeakin: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add some logging and template parsing error messages (73.76 KB, patch)
2009-12-10 08:50 PST, Neil Deakin
jonas: review+
neil: superreview-
Details | Diff | Review
screenshot of logged messages (112.44 KB, image/png)
2009-12-10 08:52 PST, Neil Deakin
no flags Details
address comments (75.56 KB, patch)
2010-01-22 13:42 PST, Neil Deakin
neil: superreview+
Details | Diff | Review

Description Neil Deakin 2005-12-21 20:15:52 PST
This involves two things:
 - log syntax errors to the console
 - provide some kind of trace of what data is being examined
Comment 1 Neil Deakin 2007-05-22 05:07:58 PDT
This is fairly easy, but waiting for bug 321171 before making more template changes.
Comment 2 Neil Deakin 2009-12-10 08:50:52 PST
Created attachment 416909 [details] [diff] [review]
add some logging and template parsing error messages

Two types of messages are output:

1. Syntax errors in template queries
2. If flags="logging" is used on the template root node, each generated item is logged to the console. This makes it easier to see that data is arriving, being generated, and matching or not matching any rules.

I'm not sure how clear the messages are but don't want the strings to be too lengthy.
Comment 3 Neil Deakin 2009-12-10 08:52:38 PST
Created attachment 416910 [details]
screenshot of logged messages
Comment 4 Neil Deakin 2009-12-10 08:57:43 PST
Note that I built this on top of the patch in bug 321169 so it may not apply perfectly without that one.
Comment 5 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2009-12-10 16:18:15 PST
(In reply to comment #4)
> Note that I built this on top of the patch in bug 321169 so it may not apply

You're wrong, this is bug 321169, you probably meant a different bug.
Comment 6 Neil Deakin 2009-12-10 16:32:03 PST
I meant bug 321177.
Comment 7 Jonas Sicking (:sicking) 2009-12-11 16:42:42 PST
Comment on attachment 416909 [details] [diff] [review]
add some logging and template parsing error messages

r=me

Would be nice to have a localization story here though.
Comment 8 neil@parkwaycc.co.uk 2010-01-11 03:51:42 PST
Comment on attachment 416909 [details] [diff] [review]
add some logging and template parsing error messages

>-        return PR_FALSE;
>+        return NS_ERROR_FAILURE;
PRBool return value (twice)

>+void
>+nsXULContentUtils::LogMessage(const char* aStr)
>+{
>+  nsAutoString message;
>+  message.AssignLiteral("Error parsing template: ");
Name the method LogTemplateError instead perhaps?

>+    printf("%s\n", NS_ConvertUTF16toUTF8(message).get());
("Error parsing template: %s\n", str) perhaps?

>+    nsTemplateMatch *newmatch = nsnull;
I assume you want to specify a specific ordering of logging. Perhaps you could log the match to be removed/replaced first, before processing the new result?

>+#ifdef PR_LOGGING
>+    // always enable logging if the debug setting is used
>+    if (PR_LOG_TEST(gXULTemplateLog, PR_LOG_DEBUG))
Except you now always send the output to stdout instead.

>-            PR_LOG(gXULTemplateLog, PR_LOG_ALWAYS,
>-                   ("xultemplate[%p] unrecognized binding <%s>",
>-                    this, tagstrC.get()));
Does this no longer get logged?

>         eDontTestEmpty = (1 << 0),
>-        eDontRecurse = (2 << 0)
>+        eDontRecurse = (2 << 0),
>+        eLoggingEnabled = (4 << 0)
1 << 0, 1 << 1, 1 << 2 etc.

>     nsCOMPtr<nsIRDFResource> pres;
>+    if (predicate.IsEmpty() || predicate[0] == PRUnichar('?')) {
>+        nsXULContentUtils::LogMessage(ERROR_TEMPLATE_TRIPLE_BAD_PREDICATE);
>         return NS_OK;
>     }
>+    else {
Don't else after return.

>-    NS_ENSURE_SUCCESS(rv, rv);
>+    if (NS_FAILED(rv)) {
>+        nsXULContentUtils::LogMessage(ERROR_TEMPLATE_BAD_XPATH);
>+        return NS_OK;
Why the change in return value?
Comment 9 Neil Deakin 2010-01-22 13:42:10 PST
Created attachment 423053 [details] [diff] [review]
address comments

Also fixes some includes that were only being includes in debug builds, and an issue with a couple of the tests.

> Except you now always send the output to stdout instead.

Changed to use PR_LOG instead.

>>-            PR_LOG(gXULTemplateLog, PR_LOG_ALWAYS,
>>-                   ("xultemplate[%p] unrecognized binding <%s>",
>>-                    this, tagstrC.get()));
>Does this no longer get logged?

As the <bindings> element is optional, it caused unnecessary warnings, so this message should just be removed.
Comment 11 Eric Shepherd [:sheppy] 2011-01-06 12:22:14 PST
Do these show up in the web console or only in the older error console?
Comment 12 Eric Shepherd [:sheppy] 2011-01-12 11:33:16 PST
Documented here:

https://developer.mozilla.org/en/XUL/Template_Guide/Debugging_XUL_templates

Linked from:

https://developer.mozilla.org/en/XUL/Template_Guide#Additional_Topics
and from Firefox 4 for developers
Comment 13 Neil Deakin 2011-01-12 11:35:12 PST
It's actually already documented in the template guide at https://developer.mozilla.org/en/XUL/Template_Guide/Template_Logging

Note You need to log in before you can comment on or make changes to this bug.