Closed Bug 321169 Opened 14 years ago Closed 10 years ago

XUL templates need better logging capability

Categories

(Core :: XUL, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: enndeakin, Assigned: enndeakin)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 1 obsolete file)

This involves two things:
 - log syntax errors to the console
 - provide some kind of trace of what data is being examined
Blocks: 330010
Blocks: 330012
This is fairly easy, but waiting for bug 321171 before making more template changes.
Depends on: 321171
Target Milestone: mozilla1.9alpha1 → mozilla1.9alpha6
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
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.
Attachment #416909 - Flags: superreview?(neil)
Attachment #416909 - Flags: review?(jonas)
Note that I built this on top of the patch in bug 321169 so it may not apply perfectly without that one.
(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.
I meant bug 321177.
Status: NEW → ASSIGNED
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.
Attachment #416909 - Flags: review?(jonas) → review+
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?
Attachment #416909 - Flags: superreview?(neil) → superreview-
Attached patch address commentsSplinter Review
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.
Attachment #416909 - Attachment is obsolete: true
Attachment #423053 - Flags: superreview?(neil)
Attachment #423053 - Flags: superreview?(neil) → superreview+
http://hg.mozilla.org/mozilla-central/rev/689d5779f815
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Blocks: 543710
Do these show up in the web console or only in the older error console?
It's actually already documented in the template guide at https://developer.mozilla.org/en/XUL/Template_Guide/Template_Logging
You need to log in before you can comment on or make changes to this bug.