XUL templates need better logging capability

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
XUL
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Neil Deakin, Assigned: Neil Deakin)

Tracking

({dev-doc-complete})

Trunk
mozilla1.9alpha8
x86
Linux
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

12 years ago
This involves two things:
 - log syntax errors to the console
 - provide some kind of trace of what data is being examined

Updated

11 years ago
Blocks: 330010
(Assignee)

Updated

11 years ago
Blocks: 330012
(Assignee)

Comment 1

10 years ago
This is fairly easy, but waiting for bug 321171 before making more template changes.
Depends on: 321171
Target Milestone: mozilla1.9alpha1 → mozilla1.9alpha6
(Assignee)

Updated

10 years ago
Target Milestone: mozilla1.9alpha6 → mozilla1.9beta1

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
(Assignee)

Comment 2

8 years ago
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.
Attachment #416909 - Flags: superreview?(neil)
Attachment #416909 - Flags: review?(jonas)
(Assignee)

Comment 3

8 years ago
Created attachment 416910 [details]
screenshot of logged messages
(Assignee)

Comment 4

8 years ago
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.
(Assignee)

Comment 6

8 years ago
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 8

7 years ago
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-
(Assignee)

Comment 9

7 years ago
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.
Attachment #416909 - Attachment is obsolete: true
Attachment #423053 - Flags: superreview?(neil)

Updated

7 years ago
Attachment #423053 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 10

7 years ago
http://hg.mozilla.org/mozilla-central/rev/689d5779f815
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED

Updated

7 years ago
Blocks: 543710
Do these show up in the web console or only in the older error console?
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
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 13

6 years ago
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.