Closed
Bug 758870
Opened 12 years ago
Closed 12 years ago
de-ns-ify nsDocAccessible
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: surkov, Assigned: capella)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++])
Attachments
(1 file, 1 obsolete file)
355.70 KB,
patch
|
Details | Diff | Splinter Review |
similar to bug 748724.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•12 years ago
|
||
Builds and tests locally, push to TRY: https://tbpl.mozilla.org/?tree=Try&rev=738741efa386 This doesn't include moving the classes into the a11y namespace ... originally I coded that change but ran into build problems involving the DocAccessible cache, so I pulled just that portion out to verify it as the cause of the problems. I'm posting it here for feedback, and wondering it we might move the patch along as-is and do a follow-up for the namespace change later ... allows us to move this large change quickly and cleanly without modifying too much code other than nsDocAccessible to DocAccessible renames ...
Attachment #627505 -
Flags: feedback?(surkov.alexander)
Reporter | ||
Comment 2•12 years ago
|
||
Comment on attachment 627505 [details] [diff] [review] Patch (v1) Review of attachment 627505 [details] [diff] [review]: ----------------------------------------------------------------- r=me, please fix style nits and land it asap ::: accessible/src/atk/nsDocAccessibleWrap.cpp @@ +16,1 @@ > nsIPresShell* aPresShell) : nit: correct indent ::: accessible/src/atk/nsDocAccessibleWrap.h @@ +15,2 @@ > > +class DocAccessibleWrap: public DocAccessible nit: space before : ::: accessible/src/base/nsAccDocManager.h @@ +32,5 @@ > > /** > * Return document accessible for the given DOM node. > */ > + DocAccessible* GetDocAccessible(nsIDocument *aDocument); nit: nsIDocument* aDocument @@ +105,5 @@ > > /** > * Create document or root accessible. > */ > + DocAccessible* CreateDocOrRootAccessible(nsIDocument *aDocument); type* name ::: accessible/src/base/nsAccUtils.h @@ +106,5 @@ > > /** > * Return document accessible for the given DOM node. > */ > + static DocAccessible* GetDocAccessibleFor(nsINode *aNode) type* name @@ +115,5 @@ > > /** > * Return document accessible for the given docshell. > */ > + static DocAccessible* GetDocAccessibleFor(nsIDocShellTreeItem *aContainer) type* name ::: accessible/src/base/nsEventShell.cpp @@ +5,5 @@ > > #include "nsEventShell.h" > > #include "nsAccUtils.h" > +//#include "DocAccessible.h" nit: pls remove the comment ::: accessible/src/base/nsDocAccessible.cpp @@ +325,5 @@ > > } > > NS_IMETHODIMP > +DocAccessible::GetAttributes(nsIPersistentProperties **aAttributes) type* name @@ +342,5 @@ > // be contained within the current document. > return FocusMgr()->FocusedAccessible(); > } > > +NS_IMETHODIMP DocAccessible::TakeFocus() nit: NS_IMETHODIMP on new line pls @@ +361,5 @@ > > //////////////////////////////////////////////////////////////////////////////// > // nsIAccessibleDocument > > +NS_IMETHODIMP DocAccessible::GetURL(nsAString& aURL) same @@ +418,5 @@ > > return NS_ERROR_FAILURE; > } > > +NS_IMETHODIMP DocAccessible::GetNameSpaceURIForID(PRInt16 aNameSpaceID, nsAString& aNameSpaceURI) same @@ +429,5 @@ > } > return NS_ERROR_FAILURE; > } > > +NS_IMETHODIMP DocAccessible::GetWindowHandle(void **aWindow) return type on own line, type* name @@ +436,5 @@ > *aWindow = GetNativeWindow(); > return NS_OK; > } > > +NS_IMETHODIMP DocAccessible::GetWindow(nsIDOMWindow **aDOMWin) same @@ +453,5 @@ > return NS_OK; > } > > NS_IMETHODIMP > +DocAccessible::GetDOMDocument(nsIDOMDocument **aDOMDocument) type* name @@ +494,1 @@ > nsIAccessibleDocument** aDocument) wrong indentation @@ +671,5 @@ > return root; > } > > +// DocAccessible protected member > +void DocAccessible::GetBoundsRect(nsRect& aBounds, nsIFrame** aRelativeFrame) return type on own line @@ +712,5 @@ > } > } > > +// DocAccessible protected member > +nsresult DocAccessible::AddEventListeners() same @@ +755,5 @@ > return NS_OK; > } > > +// DocAccessible protected member > +nsresult DocAccessible::RemoveEventListeners() same @@ +798,5 @@ > > return NS_OK; > } > > +void DocAccessible::ScrollTimerCallback(nsITimer *aTimer, void *aClosure) same plus type* name @@ +820,5 @@ > } > } > > +// DocAccessible protected member > +void DocAccessible::AddScrollListener() too many of them, it's probably worth to run through the file and fix all of them @@ +878,1 @@ > const PRUnichar *aData) wrong indentation @@ +898,2 @@ > nsIAccessible* aOldAccessible, > PRInt32 aOldStart, PRInt32 aOldEnd) same @@ +914,3 @@ > > void > +DocAccessible::AttributeWillChange(nsIDocument *aDocument, type* name @@ +953,3 @@ > dom::Element* aElement, > PRInt32 aNameSpaceID, nsIAtom* aAttribute, > PRInt32 aModType) wrong indentation @@ +1210,3 @@ > nsIContent* aContainer, > nsIContent* aFirstNewContent, > PRInt32 /* unused */) indent it @@ +1217,2 @@ > nsIContent* aContent, > nsEventStates aStateMask) same and void on own line @@ +1243,1 @@ > nsEventStates aStateMask) please run through the file and fix broken indentation ::: accessible/src/base/nsDocAccessible.h @@ +54,5 @@ > public: > using nsAccessible::GetParent; > > + DocAccessible(nsIDocument *aDocument, nsIContent *aRootContent, > + nsIPresShell* aPresShell); nit: type* name ::: accessible/src/mac/nsDocAccessibleWrap.h @@ +10,2 @@ > > +class DocAccessibleWrap: public DocAccessible nit: space before : ::: accessible/src/msaa/nsDocAccessibleWrap.cpp @@ +48,5 @@ > > //----------------------------------------------------- > // IUnknown interface methods - see iunknown.h for documentation > //----------------------------------------------------- > +STDMETHODIMP_(ULONG) DocAccessibleWrap::AddRef() return type on ow line please (through this file) ::: accessible/src/msaa/nsDocAccessibleWrap.h @@ +17,3 @@ > #include "nsIDocShellTreeItem.h" > > +class DocAccessibleWrap: public DocAccessible, nit: space before :
Attachment #627505 -
Flags: feedback?(surkov.alexander) → review+
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to Mark Capella [:capella] from comment #1) > Created attachment 627505 [details] [diff] [review] > Patch (v1) > > Builds and tests locally, push to TRY: > https://tbpl.mozilla.org/?tree=Try&rev=738741efa386 > > This doesn't include moving the classes into the a11y namespace ... > originally I coded that change but ran into build problems involving the > DocAccessible cache, so I pulled just that portion out to verify it as the > cause of the problems. > > I'm posting it here for feedback, and wondering it we might move the patch > along as-is and do a follow-up for the namespace change later ... allows us > to move this large change quickly and cleanly without modifying too much > code other than nsDocAccessible to DocAccessible renames ... fine with me. I'd say we should put DocAccessible into namespace after we put all other accessible classes (except nsAccessible perhaps) into that namespace, otherwise you need to add mozilla::a11y for every header file and then remove them.
Assignee | ||
Comment 4•12 years ago
|
||
Latest version, nits addressed ... patch merged to inbound this time ... minor changes due to clash with msaa/nsAccessibleWrap.cpp and .h class constructor / destructor ... Pushing to try ... https://tbpl.mozilla.org/?tree=Try&rev=0d202956a8a0
Attachment #627505 -
Attachment is obsolete: true
Assignee | ||
Comment 5•12 years ago
|
||
Good TRY run, pushing to inbound ... https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f41434c72cb5
Target Milestone: --- → mozilla15
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f41434c72cb5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•