Closed
Bug 721947
Opened 12 years ago
Closed 12 years ago
don't use nsIWeakShell
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: surkov, Assigned: hub)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file, 1 obsolete file)
46.74 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
when bug 672504 is fixed then we can switch to nsIPresShell, we have several instances where nsIWeakShell is used like nsAccTreeWalker and nsAccessibilityService. Hub, wanna take it?
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → hub
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
patch to be applied on top of bug 673405
Assignee | ||
Updated•12 years ago
|
Attachment #592960 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 3•12 years ago
|
||
Comment on attachment 592960 [details] [diff] [review] use nsIPresShell instead of nsIWeakShell. r= Review of attachment 592960 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/atk/nsDocAccessibleWrap.cpp @@ +47,5 @@ > //////////////////////////////////////////////////////////////////////////////// > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : 2 space indent aShell -> aPresShell and do that everywhere in the patch please ::: accessible/src/base/nsAccDocManager.cpp @@ +382,5 @@ > // XXXaaronl: ideally we would traverse the presshell chain. Since there's > // no easy way to do that, we cheat and use the document hierarchy. > // GetAccessible() is bad because it doesn't support our concept of multiple > // presshells per doc. It should be changed to use > + // GetAccessibleInShell(). get rid GetAccessibleIsShell at all, make GetAccessible take nsIPresShell as argument ::: accessible/src/base/nsAccTreeWalker.cpp @@ +69,2 @@ > bool aWalkAnonContent, bool aWalkCache) : > + mShell(aShell), mWalkCache(aWalkCache), mState(nsnull) use nsDocAccessible instead of nsIPresShell ::: accessible/src/base/nsAccessibilityService.cpp @@ +941,2 @@ > bool* aIsSubtreeHidden) > { make it take nsDocAccessible instead nsIPresShell ::: accessible/src/base/nsTextEquivUtils.cpp @@ +117,5 @@ > return NS_OK; > > gInitiatorAcc = aInitiatorAcc; > > + nsIPresShell* shell = nsCoreUtils::GetPresShellFor(aContent); it makes sense to use nsDocAcessbile instead ::: accessible/src/html/nsHTMLTableAccessible.cpp @@ +351,4 @@ > > do { > nsAccessible* headerCell = > + GetAccService()->GetAccessibleInShell(headerCellElm, presShell); use nsDocAccessible::GetAccessible() @@ +721,5 @@ > if (NS_SUCCEEDED(rv) && startRowIndex == rowIndex && > startColIndex == columnIndex && isSelected) { > nsCOMPtr<nsIContent> cellContent(do_QueryInterface(cellElement)); > nsAccessible *cell = > + GetAccService()->GetAccessibleInShell(cellContent, presShell); same @@ +895,5 @@ > NS_ENSURE_SUCCESS(rv, rv); > > nsCOMPtr<nsIContent> cellContent(do_QueryInterface(cellElement)); > nsAccessible* cell = > + GetAccService()->GetAccessibleInShell(cellContent, mDoc->PresShell()); same ::: accessible/src/xul/XULSelectControlAccessible.cpp @@ +140,5 @@ > mSelectControl->GetSelectedItem(getter_AddRefs(itemElm)); > > nsCOMPtr<nsINode> itemNode(do_QueryInterface(itemElm)); > return itemNode && mDoc ? > + GetAccService()->GetAccessibleInShell(itemNode, mDoc->PresShell()) : nsnull; nsDocAccessible::GetAccessible()
Attachment #592960 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 4•12 years ago
|
||
changing summary because actually we need to replace nsIWeakShell on nsDocAccessible
Summary: use nsIPresShell instead of nsIWeakShell → don't use nsIWeakShell
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #592960 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #595422 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 595422 [details] [diff] [review] use nsIPresShell instead of nsIWeakShell. r= Review of attachment 595422 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: accessible/public/nsIAccessibilityService.h @@ +71,5 @@ > * @param aPresShell [in] the presentation shell which contains layout info > * for the DOM node > */ > + virtual nsAccessible* GetAccessible(nsINode* aNode, > + nsIPresShell* aPresShell) = 0; move it to nsAccessibilityService, remove existing version of nsAccessibilityService::GetAccessible ::: accessible/src/atk/nsDocAccessibleWrap.cpp @@ +47,5 @@ > //////////////////////////////////////////////////////////////////////////////// > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : aShell -> aPresShell @@ +48,5 @@ > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : > + nsDocAccessible(aDocument, aRootContent, aShell), mActivated(false) aShell -> aPresShell ::: accessible/src/atk/nsDocAccessibleWrap.h @@ +50,5 @@ > class nsDocAccessibleWrap: public nsDocAccessible > { > public: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell); aShell -> aPresShell ::: accessible/src/base/nsAccessibilityService.cpp @@ +958,2 @@ > // Check to see if we already have an accessible for this node in the cache. > + nsAccessible* cachedAccessible = GetAccessible(aNode, presShell); just do aDoc->GetAccessible(aNode), it will return cached accessible if any @@ +974,5 @@ > NS_WARNING("Creating accessible for node with no document"); > return nsnull; > } > > + if (aNode->OwnerDoc() != presShell->GetDocument()) { aNode->OwnerDoc() != aDoc->GetDocumentNode() @@ +1107,5 @@ > continue; > > if (tableFrame->GetType() == nsGkAtoms::tableOuterFrame) { > nsAccessible *tableAccessible = > + GetAccessible(tableContent, presShell); aDoc->GetAccessible ::: accessible/src/base/nsDocAccessible.cpp @@ +102,5 @@ > // Constructor/desctructor > > nsDocAccessible:: > nsDocAccessible(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : aShell -> aPresShell ::: accessible/src/base/nsDocAccessible.h @@ +96,5 @@ > public: > using nsAccessible::GetParent; > > nsDocAccessible(nsIDocument *aDocument, nsIContent *aRootContent, > + nsIPresShell* aShell); aShell -> aPresShell ::: accessible/src/base/nsRootAccessible.h @@ +68,5 @@ > NS_DECL_ISUPPORTS_INHERITED > > public: > + nsRootAccessible(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell); same ::: accessible/src/mac/nsDocAccessibleWrap.h @@ +44,5 @@ > class nsDocAccessibleWrap: public nsDocAccessible > { > public: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell); same ::: accessible/src/mac/nsDocAccessibleWrap.mm @@ +40,5 @@ > #import "mozAccessible.h" > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : same ::: accessible/src/mac/nsRootAccessibleWrap.h @@ +50,5 @@ > class nsRootAccessibleWrap : public nsRootAccessible > { > public: > + nsRootAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell); same ::: accessible/src/mac/nsRootAccessibleWrap.mm @@ +46,5 @@ > #include "nsIViewManager.h" > > nsRootAccessibleWrap:: > + nsRootAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : same ::: accessible/src/msaa/nsDocAccessibleWrap.cpp @@ +68,5 @@ > //////////////////////////////////////////////////////////////////////////////// > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : same ::: accessible/src/msaa/nsDocAccessibleWrap.h @@ +53,5 @@ > public ISimpleDOMDocument > { > public: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell); same ::: accessible/src/msaa/nsRootAccessibleWrap.cpp @@ +50,5 @@ > // Constructor/desctructor > > nsRootAccessibleWrap:: > nsRootAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : same and everywhere below
Attachment #595422 -
Flags: review?(surkov.alexander) → review+
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 595422 [details] [diff] [review] use nsIPresShell instead of nsIWeakShell. r= Review of attachment 595422 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/public/nsIAccessibilityService.h @@ +71,5 @@ > * @param aPresShell [in] the presentation shell which contains layout info > * for the DOM node > */ > + virtual nsAccessible* GetAccessible(nsINode* aNode, > + nsIPresShell* aPresShell) = 0; It is this method that is implemented in nsAccessibilityService. Or maybe you just want it removed from nsIAccessibleService as well? What I was actually thinking is filing a bug to remove nsAccessibilityService::GetAccessible() completely as it requires a PresShell that we get from the nsDocAccessible, in that case all we need is nsAccDocManager::GetDocAccessible() and use that nsDocAccessible to get the accessible for the node. ::: accessible/src/atk/nsDocAccessibleWrap.cpp @@ +47,5 @@ > //////////////////////////////////////////////////////////////////////////////// > > nsDocAccessibleWrap:: > + nsDocAccessibleWrap(nsIDocument* aDocument, nsIContent* aRootContent, > + nsIPresShell* aShell) : I will do that. ::: accessible/src/base/nsAccessibilityService.cpp @@ +958,2 @@ > // Check to see if we already have an accessible for this node in the cache. > + nsAccessible* cachedAccessible = GetAccessible(aNode, presShell); ok
Reporter | ||
Comment 8•12 years ago
|
||
(In reply to Hub Figuiere [:hub] from comment #7) > > + virtual nsAccessible* GetAccessible(nsINode* aNode, > > + nsIPresShell* aPresShell) = 0; > > It is this method that is implemented in nsAccessibilityService. Or maybe > you just want it removed from nsIAccessibleService as well? right > What I was actually thinking is filing a bug to remove > nsAccessibilityService::GetAccessible() completely as it requires a > PresShell that we get from the nsDocAccessible, in that case all we need is > nsAccDocManager::GetDocAccessible() and use that nsDocAccessible to get the > accessible for the node. that's right, up to you if you don't want to remove it from nsIAccessibilityService right now.
Assignee | ||
Comment 9•12 years ago
|
||
File bug 725572 to do that cleanup in comment 8
Assignee | ||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a93dc11d7b7
Target Milestone: --- → mozilla13
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7a93dc11d7b7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•