543710 - Logging for XUL template based on storage

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[Laurent Jouanneau]

We have now logging capability into the xul template engine (bug 321169). The storage query processor should use this logging system, to inform about errors which could appear (no database, bad sql query etc..).

I'm working on a patch.

Comment 1

[Laurent Jouanneau]

Attachment: The patch

Comment 2

[[e-mail]]

Comment on attachment 425191 [details] [diff] [review]
The patch

>+#define ERROR_TEMPLATE_STORAGE_UNKNOW_QUERY_PARAMETER                   \
Typo: _UNKNOWN_

>+            if (typeError < 1) {
Nit: I'd prefer <= 0

Comment 3

[Neil Deakin]

Comment on attachment 425191 [details] [diff] [review]
The patch

>+#define ERROR_TEMPLATE_STORAGE_BAD_URI                                  \
>+        "Only profile: or file URI are allowed"
>+#define ERROR_TEMPLATE_STORAGE_CANNOT_OPEN_DATABASE                     \
>+        "Cannot open given database"
>+#define ERROR_TEMPLATE_STORAGE_BAD_QUERY                                \
>+        "Syntax error in the SQL query"
>+#define ERROR_TEMPLATE_STORAGE_UNKNOW_QUERY_PARAMETER                   \
>+        "The given named parameter is unknown in the SQL query"
>+#define ERROR_TEMPLATE_STORAGE_WRONG_TYPE_QUERY_PARAMETER               \
>+        "The type of a query parameter is wrong"
>+#define ERROR_TEMPLATE_STORAGE_QUERY_PARAMETER_NOT_BOUND                \
>+        "A query parameter cannot be bound to the SQL query"

The existing errors begin with a lowercase letter.


>+expectedConsoleMessages.push("Error parsing template: The given named parameter is unknown in the SQL query");
>+expectedConsoleMessages.push("Error parsing template: The type of a query parameter is wrong");
>+expectedConsoleMessages.push("Error parsing template: A query parameter cannot be bound to the SQL query");

Although it's probably ok, this relies on the templates being parsed in order, which isn't necessarily true. You may want to at least add a note here about this.

Comment 4

[Laurent Jouanneau]

Attachment: The patch to commit [Checkin: Comment 6]

I split the test into three files, so we won't have a potential problem with the order of messages.

Comment 5

[Laurent Jouanneau]

tinderboxes status is currently orange...

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 430718 [details] [diff] [review]
The patch to commit
[Checkin: Comment 6]


http://hg.mozilla.org/mozilla-central/rev/1cb03235231e