Closed
Bug 760099
Opened 12 years ago
Closed 12 years ago
add logging for events/test_docload.xul
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: surkov, Assigned: surkov)
References
Details
Attachments
(1 file)
17.32 KB,
patch
|
tbsaunde
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #628718 -
Flags: review?(trev.saunders)
Comment 1•12 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•12 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 3•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e65e082823d7
OS: Mac OS X → All
Target Milestone: --- → mozilla15
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e65e082823d7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Assignee: nobody → surkov.alexander
You need to log in
before you can comment on or make changes to this bug.
Description
•