Closed
Bug 873450
Opened 11 years ago
Closed 11 years ago
Implement IA2 containing relations
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: surkov, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: access, Whiteboard: [mentor=surkov.alexander@gmail.com][lang=c++])
Attachments
(2 files, 1 obsolete file)
10.10 KB,
patch
|
Details | Diff | Splinter Review | |
12.21 KB,
patch
|
tbsaunde
:
review+
|
Details | Diff | Splinter Review |
IA2_RELATION_CONTAINING_DOCUMENT IA2_RELATION_CONTAINING_TAB_PANE IA2_RELATION_CONTAINING_WINDOW IA2_RELATION_CONTAINING_APPLICATION
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 1•11 years ago
|
||
(In reply to alexander :surkov from comment #0) 1) add constants into nsIAccessibleRelation 2) implement them in Accessible::RelationByType (see below) 3) add mapping into windows/msaa/IAccessibleRelation.h/cpp 4) add tests into mochitest/relations/test_tabbrowser.xul > IA2_RELATION_CONTAINING_DOCUMENT Accessible::Document() > IA2_RELATION_CONTAINING_TAB_PANE IServiceProvider::QueryService SID_IAccessibleContentDocument, move the logic into Accessible::RelationByType > IA2_RELATION_CONTAINING_WINDOW similar to previous item, you need to get root doc shell and then get associated document accessible > IA2_RELATION_CONTAINING_APPLICATION ApplicationAcc()
Whiteboard: [mentor=surkov.alexander@gmail.com][lang=c++]
Hi I am interested in working on this bug,but it's my first time to work on with debug,can anybody guide me on how to get started with it?Thanks a lot.
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to MikeLing from comment #2) > Hi I am interested in working on this bug,but it's my first time to work on > with debug,can anybody guide me on how to get started with it?Thanks a lot. If you don't have good c++ skills then it's better to start from [good first bug]. (Note, I answered you in bug 877985 as well)
(In reply to alexander :surkov from comment #3) > (In reply to MikeLing from comment #2) > > Hi I am interested in working on this bug,but it's my first time to work on > > with debug,can anybody guide me on how to get started with it?Thanks a lot. > > If you don't have good c++ skills then it's better to start from [good first > bug]. (Note, I answered you in bug 877985 as well) Thanks for reply,I think a some experience in C++ as a junior college student.And I also know some about python,Can you tell me more about the bug or how can I get the source code?Thanks a lot
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to MikeLing from comment #4) > (In reply to alexander :surkov from comment #3) > > (In reply to MikeLing from comment #2) > > > Hi I am interested in working on this bug,but it's my first time to work on > > > with debug,can anybody guide me on how to get started with it?Thanks a lot. > > > > If you don't have good c++ skills then it's better to start from [good first > > bug]. (Note, I answered you in bug 877985 as well) > > Thanks for reply,I think a some experience in C++ as a junior college > student.And I also know some about python, ok, if you didn't work before in large projects then it's still worth to start from good first bugs. anyway up to you > Can you tell me more about the bug see comment #1 (use mxr.mozilla.com to search through sources) > or how can I get the source code?Thanks a lot https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
(In reply to alexander :surkov from comment #5) Sir,I've build the environment and get the source code already.I also check the code from mozilla-source\mozilla-central\accessible\public\nsIAccessible.idl. Should I add those arguments in comment0 to function nsIAccessibleRelation getRelationByType(in unsigned long aRelationType) directly?Because I haven't use IAccessible2 API or file.idl before.So I really need help from you.Thank you very much.
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to MikeLing from comment #6) > Should I add those arguments in comment0 to function nsIAccessibleRelation > getRelationByType(in unsigned long aRelationType) directly? no, you should add new constants into nsIAccessibleRelation.idl
(In reply to alexander :surkov from comment #7) I got some problem when I build mozilla with ./match build,It says ImportError: DLL load failed: %1 is not a valid Win32 application.but I'm sure that I put python path in my system path.What should I do?
(In reply to MikeLing from comment #8) And I'm not very clear about about new constants.
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to MikeLing from comment #8) > (In reply to alexander :surkov from comment #7) > I got some problem when I build mozilla with ./match build,It says > ImportError: DLL load failed: %1 is not a valid Win32 application.but I'm > sure that I put python path in my system path.What should I do? it's better to ask question on #developers or #build (In reply to MikeLing from comment #9) > (In reply to MikeLing from comment #8) > And I'm not very clear about about new constants. you need to add 4 constants like RELATION_CONTAINING_DOCUMENT (see comment #0), check nsIAccessibleRelation to see how other relation constants are defined there.
Comment 11•11 years ago
|
||
(In reply to alexander :surkov from comment #10) Sorry,I fell it's too hard to me right now.
Assignee | ||
Comment 12•11 years ago
|
||
(In reply to MikeLing from comment #11) > (In reply to alexander :surkov from comment #10) > Sorry,I fell it's too hard to me right now. if it's your first time working with mozilla sources then it's worth to start from good first bugs like https://bugzilla.mozilla.org/buglist.cgi?list_id=6719779&resolution=---&query_based_on=good-first-bug&status_whiteboard_type=substring&query_format=advanced&status_whiteboard=[good%20first%20bug]&component=Disability%20Access%20APIs&product=Core&known_name=good-first-bug
Comment 13•11 years ago
|
||
Initial patch minus tests. Feedback welcomed.
Assignee: nobody → andrew.quartey
Attachment #770904 -
Flags: feedback?(surkov.alexander)
Assignee | ||
Comment 14•11 years ago
|
||
Comment on attachment 770904 [details] [diff] [review] WIP Review of attachment 770904 [details] [diff] [review]: ----------------------------------------------------------------- please add navRelations stuffs ::: accessible/public/nsIAccessibleRelation.idl @@ +114,5 @@ > + * IAccessibleDocument interface. > + */ > + const unsigned long RELATION_CONTAINING_DOCUMENT = 0x11; > + > +/** nit: two spaces indent, here and below @@ +125,5 @@ > + */ > + const unsigned long RELATION_CONTAINING_WINDOW = 0x13; > + > +/** > + * The target object is the containing application object. nit: whitespace in the end of line ::: accessible/src/generic/Accessible.cpp @@ +2197,5 @@ > + IID_IAccessible, (void**) &docAcc); > + if (SUCCEEDED(hr)) > + return Relation(docAcc, mContent); > + } > + return Relation(); it should be done in vise versa way, i.e. IServiceProvider should be implemented via RelationsByType()
Attachment #770904 -
Flags: feedback?(surkov.alexander)
Comment 15•11 years ago
|
||
Fixed nits and added the navRelations bits. Also, since the implementations for |RELATION_CONTAINING_TAB_PANE| and |RELATION_CONTAINING_WINDOW| are nearly identical - they differ by only by the calls to GetSameTypeRootTreeItem and GetRootTreeItem- i can probably refactor that into a common function. Patch sent to Try: https://tbpl.mozilla.org/?tree=Try&rev=99b9bc08e6a2
Attachment #770904 -
Attachment is obsolete: true
Attachment #771468 -
Flags: feedback?(surkov.alexander)
Assignee | ||
Comment 16•11 years ago
|
||
Comment on attachment 771468 [details] [diff] [review] WIP Review of attachment 771468 [details] [diff] [review]: ----------------------------------------------------------------- you didn't change IServiceProvider implementation, was it intentional?
Assignee | ||
Comment 17•11 years ago
|
||
Comment on attachment 771468 [details] [diff] [review] WIP canceling feedback request until comment #16 answered
Attachment #771468 -
Flags: feedback?(surkov.alexander)
Comment 18•11 years ago
|
||
Unassigning myself to allow someone else to work on this.
Assignee: andrew.quartey → nobody
Assignee | ||
Comment 19•11 years ago
|
||
1) no window relation since it's currently MSAA specific, no easy way to add but AT has easy way to get the window having HWND 2) tab pane relation returns a content document what doesn't seem follow the spec. It's friendly with ServiceProvider corresponding IID and friendly to content in own process architecture. If this impl is rather a problem then fix it then
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #814150 -
Flags: review?(trev.saunders)
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to alexander :surkov from comment #19) > Created attachment 814150 [details] [diff] [review] > patch > > 1) no window relation since it's currently MSAA specific, no easy way to add > but AT has easy way to get the window having HWND or probably it makes sense to add a constant but return nullptr. Let me know what you think is right/better
Comment 21•11 years ago
|
||
Comment on attachment 814150 [details] [diff] [review] patch >+ * The target object is the containing document object which implements >+ * IAccessibleDocument interface. >+ */ >+ const unsigned long RELATION_CONTAINING_DOCUMENT = 0x11; seems funny to talk about ia2 thing here >+ >+ /** >+ * The target object is the containing tab pane object. >+ */ >+ const unsigned long RELATION_CONTAINING_TAB_PANE = 0x12; I'd think it would be more useful to say tab document or something at least that would seem more consistant with other comments >+ >+ /** >+ * The target object is the containing application object. >+ */ >+ const unsigned long RELATION_CONTAINING_APPLICATION = 0x14; wouldn't application accessible be more consistant? >+ >+ /** >+ * The target object is the containing document object. >+ */ >+ CONTAINING_DOCUMENT = 0x11, same for this file btw why don't we put comments in map file and generate this one?
Attachment #814150 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #21) > >+ * The target object is the containing application object. > >+ */ > >+ const unsigned long RELATION_CONTAINING_APPLICATION = 0x14; > > wouldn't application accessible be more consistant? it seems all constant descriptions use 'object' is used instead 'accessible' > btw why don't we put comments in map file and generate this one? I like to have constants not generated
Comment 23•11 years ago
|
||
(In reply to alexander :surkov from comment #22) > (In reply to Trevor Saunders (:tbsaunde) from comment #21) > > > >+ * The target object is the containing application object. > > >+ */ > > >+ const unsigned long RELATION_CONTAINING_APPLICATION = 0x14; > > > > wouldn't application accessible be more consistant? > > it seems all constant descriptions use 'object' is used instead 'accessible' THAT'S SAID i THINK IT WOULD BE MORE NATURAL / CONSISTANT WITH OTHER THINGS TO USE ACCESSIBLE. > > btw why don't we put comments in map file and generate this one? > > I like to have constants not generated WHY?
Assignee | ||
Comment 24•11 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #23) > THAT'S SAID i THINK IT WOULD BE MORE NATURAL / CONSISTANT WITH OTHER THINGS > TO USE ACCESSIBLE. I agree but if rename them then at once > > > btw why don't we put comments in map file and generate this one? > > > > I like to have constants not generated > > WHY? constants is a primary thing, map is a secondary thing, it seems to be a natural order of things
Assignee | ||
Comment 25•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f85c0d3d7f31
Assignee | ||
Updated•11 years ago
|
Flags: in-testsuite+
Comment 26•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f85c0d3d7f31
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•