321169 - XUL templates need better logging capability

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Neil Deakin]

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]

This is fairly easy, but waiting for bug 321171 before making more template changes.

Comment 2

[Neil Deakin]

Attachment: 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]

Attachment: screenshot of logged messages

Comment 4

[Neil Deakin]

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 (dead)]

(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]

I meant bug 321177.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

[[e-mail]]

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]

Attachment: 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 10

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/689d5779f815

Comment 11

[Eric Shepherd [:sheppy]]

Do these show up in the web console or only in the older error console?

Comment 12

[Eric Shepherd [:sheppy]]

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]

It's actually already documented in the template guide at https://developer.mozilla.org/en/XUL/Template_Guide/Template_Logging