add logging for events/test_docload.xul

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 628718 [details] [diff] [review]
patch
Attachment #628718 - Flags: review?(trev.saunders)

Comment 1

5 years ago
Comment on attachment 628718 [details] [diff] [review]
patch

Some drive-by nits I found:

>- * Log the accesisble object address (4 spaces indent).
>+ * Log the accesisble object address as message entry (4 spaces indent).

Correct spelling of "accessible" while you're here.

>+function enableLogging(aModules)
>+{
>+  gAccRetrieval.setLogging(aModules);
>+}
>+function disableLogging()
>+{
>+  gAccRetrieval.setLogging("");
>+}
>+

Blank line between the two functions.
(Assignee)

Comment 2

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #1)
> >+function enableLogging(aModules)
> >+{
> >+  gAccRetrieval.setLogging(aModules);
> >+}
> >+function disableLogging()
> >+{
> >+  gAccRetrieval.setLogging("");
> >+}
> >+
> 
> Blank line between the two functions.

they share the same comment so I keep them together intentionally.
Comment on attachment 628718 [details] [diff] [review]
patch

>+  if (type == nsISelectionController::SELECTION_NORMAL ||
>+      type == nsISelectionController::SELECTION_SPELLCHECK) {
>+
>+    bool isNormalSelection =
>+      (type == nsISelectionController::SELECTION_NORMAL);
>+
>+    bool isIgnored = !aDocument || !aDocument->IsContentLoaded();
>+    printf("\nSelection changed, selection type: %s, notification %s\n",
>+           (isNormalSelection ? "normal" : "spellcheck"),
>+           (isIgnored ? "ignored" : "pending"));
>+  } else {
>+    bool isIgnored = !aDocument || !aDocument->IsContentLoaded();
>+    printf("\nSelection changed, selection type: unknown, notification %s\n",
>+           (isIgnored ? "ignored" : "pending"));
>+  }

it seems like this might be simpler if we did if type == normal ... else if type == spellcheck ... else ...

>+#ifdef DEBUG
>+  if (eventCount > 0) {
>+    if (logging::IsEnabled(logging::eEvents)) {
>+      logging::MsgBegin("EVENTS", "events processing");
>+      logging::Address("document", mDocument);
>+      logging::MsgEnd();
>+    }
>+  }
>+#endif

I'd probably make that one if, and put a blank line after the ifdef to seperate it from real code.
Attachment #628718 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e65e082823d7
OS: Mac OS X → All
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/e65e082823d7
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Assignee: nobody → surkov.alexander
You need to log in before you can comment on or make changes to this bug.