Closed
Bug 724452
Opened 12 years ago
Closed 12 years ago
use nsFocusManager::GetFocusManager() more
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: tbsaunde, Assigned: jhk)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++])
Attachments
(1 file, 2 obsolete files)
4.85 KB,
patch
|
hub
:
review+
|
Details | Diff | Splinter Review |
look for cases of nsCOMPtr<nsIFocusManager> fm = do_GetService(NS_FOCUSMANAGER_CONTRACTID) using grep or http://mxr.mozilla.org/mozilla-central/ and replace them with nsFocusManager* fm = nsFocusManager::GetFocusManager();
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → jigneshhk1992
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #595671 -
Flags: review?(trev.saunders)
Reporter | ||
Comment 2•12 years ago
|
||
Comment on attachment 595671 [details] [diff] [review] Patch r=me for accessible/ part, but I'm not a peer for the rest of these things, so you need to find someone else to review that or only do the a11y part.
Attachment #595671 -
Flags: review?(trev.saunders) → review+
Comment 3•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #2) > Comment on attachment 595671 [details] [diff] [review] > Patch > > r=me for accessible/ part, but I'm not a peer for the rest of these things, > so you need to find someone else to review that or only do the a11y part. I suspect everything outside of accessible part might be wrong because nsFocusManager::GetFocusManager() doesn't make sure the nsFocusManager instance is created so I assume one of do_GetService calls can do this. We are safe to assume on accessibility side that GetFocusManager is not null because DOM focus happens prior we can touch focus stuff in a11y.
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #595671 -
Attachment is obsolete: true
Attachment #595997 -
Flags: review?(trev.saunders)
Comment 5•12 years ago
|
||
Comment on attachment 595997 [details] [diff] [review] Patch modified for a11y. Review of attachment 595997 [details] [diff] [review]: ----------------------------------------------------------------- moving review request to Hub (Trevor agreed to steal review request from him per irc)
Attachment #595997 -
Flags: review?(trev.saunders) → review?(hub)
Comment 6•12 years ago
|
||
Comment on attachment 595997 [details] [diff] [review] Patch modified for a11y. Review of attachment 595997 [details] [diff] [review]: ----------------------------------------------------------------- r=me Looks good for checkin
Attachment #595997 -
Flags: review?(hub) → review+
Comment 7•12 years ago
|
||
Comment on attachment 595997 [details] [diff] [review] Patch modified for a11y. Review of attachment 595997 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment fixed ::: accessible/src/base/nsAccessNode.cpp @@ +423,5 @@ > nsIDOMWindow* win = doc->GetWindow(); > > nsCOMPtr<nsIDOMWindow> focusedWindow; > nsCOMPtr<nsIDOMElement> focusedElement; > + nsFocusManager* fm = nsFocusManager::GetFocusManager(); please remove nsAccessNode::GetCurrentFocus at all since it's not used
Attachment #595997 -
Flags: review+
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #595997 -
Attachment is obsolete: true
Comment 9•12 years ago
|
||
Comment on attachment 596066 [details] [diff] [review] Removed nsAccessNode::GetCurrentFocus Good catch from Alex. Looks good this way too.
Attachment #596066 -
Flags: review+
Comment 10•12 years ago
|
||
inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/e47e25d0a6c1
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e47e25d0a6c1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in
before you can comment on or make changes to this bug.
Description
•