Open
Bug 750025
Opened 12 years ago
Updated 2 years ago
clean up nsAccessNodeWrap::MakeAccessNode()
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
NEW
People
(Reporter: tbsaunde, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
1.28 KB,
patch
|
Details | Diff | Splinter Review | |
1.42 KB,
patch
|
Details | Diff | Splinter Review |
rm a QI to nsIContent, and do a little cleanup while there.
Reporter | ||
Comment 1•12 years ago
|
||
Attachment #619365 -
Flags: review?(surkov.alexander)
Comment 2•12 years ago
|
||
Comment on attachment 619365 [details] [diff] [review] patch Review of attachment 619365 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/msaa/nsAccessNodeWrap.cpp @@ +426,4 @@ > > newNode->Init(); > iNode = static_cast<ISimpleDOMNode*>(newNode); > iNode->AddRef(); you need to fix indentation since you move this block out of 'else if' construction aNode->AsElement() should make us crash in case of inaccessbile document or text node since they aren't inherited from Element class.
Attachment #619365 -
Flags: review?(surkov.alexander)
Reporter | ||
Comment 3•12 years ago
|
||
> ::: accessible/src/msaa/nsAccessNodeWrap.cpp
> @@ +426,4 @@
> >
> > newNode->Init();
> > iNode = static_cast<ISimpleDOMNode*>(newNode);
> > iNode->AddRef();
>
> you need to fix indentation since you move this block out of 'else if'
> construction
>
> aNode->AsElement() should make us crash in case of inaccessbile document or
> text node since they aren't inherited from Element class.
yeah, I guess the assertion we had there before wasn't really correct. However we still shouldn't crash because ISimpleDOMNode methods should handle case of defunct content and I think most do.
Comment 4•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #3) > yeah, I guess the assertion we had there before wasn't really correct. what's wrong with it?
Reporter | ||
Comment 5•12 years ago
|
||
(In reply to alexander :surkov from comment #4) > (In reply to Trevor Saunders (:tbsaunde) from comment #3) > > yeah, I guess the assertion we had there before wasn't really correct. > > what's wrong with it? I thought in some cases we intentionally don't create accessible documents for dom docs? for example temporary ones or svg?
Comment 6•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #5) > I thought in some cases we intentionally don't create accessible documents > for dom docs? for example temporary ones or svg? yes, we don't create them in some cases but it doesn't prevent an AT to access to the content by ISimpleDOMNode.
Reporter | ||
Comment 7•12 years ago
|
||
(In reply to alexander :surkov from comment #6) > (In reply to Trevor Saunders (:tbsaunde) from comment #5) > > > I thought in some cases we intentionally don't create accessible documents > > for dom docs? for example temporary ones or svg? > > yes, we don't create them in some cases but it doesn't prevent an AT to > access to the content by ISimpleDOMNode. yes, and that would mean that the assert wasn't quiet right wouldn't it since the assert would fire and we'd not give t an ISimpleDOMNode? I'd think technical in this case of inaccessible document we should return object that is a ISimpleDOMDOcument as well as a ISimpleDOMNode, but that might take work and no at seems to care.
Reporter | ||
Comment 8•12 years ago
|
||
Attachment #619877 -
Flags: review?(surkov.alexander)
Comment 9•12 years ago
|
||
Comment on attachment 619877 [details] [diff] [review] patch v2 Review of attachment 619877 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/msaa/nsAccessNodeWrap.cpp @@ +424,3 @@ > > + if (!aNode->IsElement()) > + return nsnull; you ignore text nodes
Comment 10•12 years ago
|
||
Comment on attachment 619877 [details] [diff] [review] patch v2 canceling request until comments are addressed
Attachment #619877 -
Flags: review?(surkov.alexander)
Comment 11•12 years ago
|
||
It will be fixed by bug 767756.
Updated•7 years ago
|
Assignee: tbsaunde+mozbugs → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•