Bug 574336 (dexpcoma11y)

[meta] De-XPCOM accessible classes

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: surkov, Assigned: tbsaunde)

Tracking

(Blocks 3 bugs, {access, meta, perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: bk1)

(Reporter)

Description

9 years ago
We need to separate XPCOM interface methods implementation and platform specific classes implementation from core logic. So we would have core classes and XPCOM, MSAA, IA2, ATK wrap classes that redirect methods calls to core classes (via aggregation). 

As result we get something like

class Accessible
{
public:
  Accessible* GetParent() const { return mParent; }
};

class nsAccessible : public nsIAccessible
{
  NS_IMETHODIMP GetParent(nsIAccessible* aParent)
  {
     NS_IF_ADDREF(*aParent = GetXPCOMWrap(mAccessible->GetParent()));
     return NS_OK;
  }
private:
  Accessible* mAccessible;
};

class CAccessible : public IAccessible
{
public:
  hresult get_accParent()
  {
     //blabla
  }
private:
  Accessible* mAccessible;
};

So we have simpler and friendly non-XPCOM like core classes, we don't run XPCOM or MSAA until XPCOM or MSAA was asked.

The one thing that should be figured our how to get quick wrap object by core object and don't spend much memory for this.
(In reply to comment #0)
> class Accessible

What is this class responsible for?
(Reporter)

Comment 2

9 years ago
for all logic, it is what's nsAccessible is currently (expect XPCOM interfaces implementation)
Alexander, it would be good to brain dump what we discussed with Neil at the summit. Your idea made a lot of sense.
(Reporter)

Comment 4

9 years ago
Sure. We told about XPCOM interfaces impl only. There are two things we should add here:
1) de-xpcom accessible events: create xpcom event wrapper iif there is proper observer listener
2) GetXPCOMWrap should look into cache before to create xpcom wrapper for accessible
(Reporter)

Updated

9 years ago
Depends on: 589145
(Reporter)

Comment 5

9 years ago
Further thoughts.

We need to provide query mechanism like Node::GetAsElement() or downcast_Accessible like we have for accessible events. I would like to avoid multiple inheritance so that we need to line up our hierarchy. Other option would be internal interfaces creation (virtual classes), that would be nice (and personally I lean towards to this approach) but currently we deal with implementation classes rather than interfaces and therefore it doesn't make sense.

For example, nsAccessible is inherited from nsIAccessibleSelectable, nsIAccessibleValue and nsIAccessibleHyperLink. It doesn't make sense to create internal interfaces for these xpcom interfaces because we deal with nsAccessible and it's perfectly casted to these interfaces.

The easiest way to keep all methods of these interfaces in nsAccessible (like we have now) and provide methods like IsHyperLink() and etc. It's not savvy like having a interfaces (since we can forgot to check IsHyperLink() before we call proper methods). But again it doesn't require lot of changes. We might want to deal with multiple inheritance from virtual classes in the future.

Concerning table and image accessibles implementation we could insert virtual class into hierarchy like
class TableAccessible : public nsAccessible
{
  virtual void foo() = 0;
};

class nsARIAGridAccessible : public TableAccessible
{
  virtual void foo();
};
class nsHTMLTableAccessible : public TableAccessible { /* */ }

So technically it'll be similar to what we have now and we can move out xpcom interfaces implementation from main code once we have downcasting from nsAccessible to TableAccessible.
(Reporter)

Updated

9 years ago
Depends on: 589399
(Reporter)

Updated

9 years ago
Keywords: perf
(Reporter)

Updated

9 years ago
Alias: dexpcoma11y
(Reporter)

Updated

9 years ago
Depends on: 590176
(Reporter)

Updated

9 years ago
Depends on: 592193
(Reporter)

Updated

8 years ago
Keywords: meta
Summary: De-XPCOM accessible classes → [meta] De-XPCOM accessible classes
(Reporter)

Updated

8 years ago
Depends on: 634218
(Reporter)

Updated

8 years ago
Depends on: 636945
(Reporter)

Updated

8 years ago
Depends on: 641562
(Reporter)

Updated

8 years ago
Depends on: 641838
(Reporter)

Updated

8 years ago
Depends on: 643711
(Reporter)

Updated

8 years ago
Depends on: 648265
(Reporter)

Updated

8 years ago
Depends on: 648267
(Reporter)

Updated

8 years ago
Depends on: 648273
(Assignee)

Updated

8 years ago
Depends on: 652378
(Reporter)

Updated

8 years ago
Depends on: 657719
(Assignee)

Updated

8 years ago
Depends on: 671926
(Assignee)

Updated

8 years ago
Depends on: 671991
(Reporter)

Updated

8 years ago
Depends on: 673689

Updated

8 years ago
Assignee: nobody → trev.saunders

Updated

8 years ago
Whiteboard: bk1
(Assignee)

Updated

7 years ago
Depends on: 717683
(Assignee)

Updated

7 years ago
Depends on: 717689
(Assignee)

Updated

7 years ago
Depends on: 726287
(Reporter)

Updated

7 years ago
Depends on: 733335
(Assignee)

Updated

7 years ago
Depends on: 739568
(Assignee)

Updated

7 years ago
Depends on: 739883
(Assignee)

Updated

7 years ago
Depends on: 739882
(Assignee)

Updated

7 years ago
Depends on: 739884
(Assignee)

Updated

7 years ago
Depends on: 739885
(Reporter)

Updated

7 years ago
Depends on: 740375
(Reporter)

Updated

7 years ago
Depends on: 740702
Depends on: 740725
(Reporter)

Updated

7 years ago
Depends on: 740747
(Reporter)

Updated

7 years ago
Depends on: 740758
(Reporter)

Updated

7 years ago
Depends on: 740766
(Assignee)

Updated

7 years ago
Depends on: 741697
(Reporter)

Updated

7 years ago
Depends on: 741702
(Reporter)

Updated

7 years ago
Depends on: 741703
(Assignee)

Updated

7 years ago
Depends on: 741707
(Reporter)

Updated

7 years ago
Depends on: 745429
No longer depends on: 740747
(Assignee)

Updated

7 years ago
Depends on: 747219
(Assignee)

Updated

7 years ago
Depends on: 747227
(Assignee)

Updated

7 years ago
Depends on: 748639
(Reporter)

Updated

7 years ago
Depends on: 757503
(Reporter)

Updated

7 years ago
Depends on: 757504
(Assignee)

Updated

7 years ago
Depends on: 760878
(Assignee)

Updated

7 years ago
Depends on: 760880
(Assignee)

Updated

7 years ago
Depends on: 760881
(Reporter)

Updated

7 years ago
Depends on: 762038
Depends on: 765371
Depends on: 765512
Depends on: 767705
(Assignee)

Updated

7 years ago
Depends on: 768461
(Assignee)

Updated

7 years ago
Depends on: 777117
(Assignee)

Updated

7 years ago
Depends on: 787308
(Assignee)

Updated

7 years ago
Depends on: 795192
(Reporter)

Updated

7 years ago
Depends on: 799909
(Assignee)

Updated

6 years ago
Depends on: 829382
(Reporter)

Updated

6 years ago
Depends on: 932789
(Reporter)

Updated

6 years ago
Depends on: 934039
(Reporter)

Updated

6 years ago
Depends on: 935698
(Reporter)

Updated

5 years ago
Depends on: 1064877
(Reporter)

Updated

5 years ago
Depends on: 1068734
(Reporter)

Updated

5 years ago
Depends on: 1076816
(Reporter)

Comment 6

4 years ago
we've done with DeXPOMination, next step will be moving to WebIDL but that's different story
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.