Last Comment Bug 759645 - enable extended logging for a11y tree and text changes
: enable extended logging for a11y tree and text changes
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-29 21:57 PDT by alexander :surkov
Modified: 2012-06-02 18:14 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (16.96 KB, patch)
2012-05-29 21:57 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description alexander :surkov 2012-05-29 21:57:28 PDT
Created attachment 628217 [details] [diff] [review]
patch
Comment 1 Trevor Saunders (:tbsaunde) 2012-05-31 14:23:21 PDT
Comment on attachment 628217 [details] [diff] [review]
patch

>+  va_start(argptr, aEntryText);
>+  vfprintf(stdout, aEntryText, argptr);

nit, what's wrong with vprintf()?

>@@ -76,26 +80,36 @@ void OuterDocDestroy(OuterDocAccessible*
>  * Log the message ('title: text' format) on new line. Print the start and end
>  * boundaries of the message body designed by '{' and '}' (2 spaces indent for
>  * body).
>  */
> void MsgBegin(const char* aTitle, const char* aMsgText, ...);
> void MsgEnd();
> 
> /**
>+ * Log the entry into message body (4 spaces offset).

this comment and the previous one are inconsistant.

> // Uncomment to log notifications processing.
> //#define DEBUG_NOTIFICATIONS

still needed for something?

>+    logging::Node("container", aContainer);
>+    for (nsIContent* child = aStartChild; child != aEndChild;
>+         child = child->GetNextSibling()) {
>+      logging::Node("content", child);
>+    }
>+    logging::MsgEnd();

nit, blank line after }?
Comment 2 alexander :surkov 2012-05-31 19:18:33 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #1)

> >+  va_start(argptr, aEntryText);
> >+  vfprintf(stdout, aEntryText, argptr);
> 
> nit, what's wrong with vprintf()?

nothing, I just was used the first thing I found

> >@@ -76,26 +80,36 @@ void OuterDocDestroy(OuterDocAccessible*
> >  * Log the message ('title: text' format) on new line. Print the start and end
> >  * boundaries of the message body designed by '{' and '}' (2 spaces indent for
> >  * body).

> > /**
> >+ * Log the entry into message body (4 spaces offset).
> 
> this comment and the previous one are inconsistant.

they are, body designated by { } is 2 spaces, entry inside body is 4 spaces

> > // Uncomment to log notifications processing.
> > //#define DEBUG_NOTIFICATIONS
> 
> still needed for something?

yes, there's a review request in your queue

> >+    logging::Node("container", aContainer);
> >+    for (nsIContent* child = aStartChild; child != aEndChild;
> >+         child = child->GetNextSibling()) {
> >+      logging::Node("content", child);
> >+    }
> >+    logging::MsgEnd();
> 
> nit, blank line after }?

no, just keeping the whole message without empty lines. it looks ok :)
Comment 3 Trevor Saunders (:tbsaunde) 2012-05-31 21:23:50 PDT
> > >@@ -76,26 +80,36 @@ void OuterDocDestroy(OuterDocAccessible*
> > >  * Log the message ('title: text' format) on new line. Print the start and end
> > >  * boundaries of the message body designed by '{' and '}' (2 spaces indent for
> > >  * body).
> 
> > > /**
> > >+ * Log the entry into message body (4 spaces offset).
> > 
> > this comment and the previous one are inconsistant.
> 
> they are, body designated by { } is 2 spaces, entry inside body is 4 spaces

ok, that didn't make sense to me from the comments
Comment 4 alexander :surkov 2012-05-31 21:34:07 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #3)

> > they are, body designated by { } is 2 spaces, entry inside body is 4 spaces
> 
> ok, that didn't make sense to me from the comments

I see. perhaps not big deal (not sure how to make them clear enough)

Note You need to log in before you can comment on or make changes to this bug.