Closed Bug 574336 (dexpcoma11y) Opened 10 years ago Closed 5 years ago

[meta] De-XPCOM accessible classes

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: tbsaunde)

References

(Blocks 3 open bugs)

Details

(Keywords: access, meta, perf, Whiteboard: bk1)

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?
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.
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
Depends on: 589145
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.
Depends on: 589399
Keywords: perf
Alias: dexpcoma11y
Depends on: 590176
Depends on: 592193
Keywords: meta
Summary: De-XPCOM accessible classes → [meta] De-XPCOM accessible classes
Depends on: 634218
Depends on: 636945
Depends on: 641562
Depends on: 641838
Depends on: 643711
Depends on: 648265
Depends on: 648267
Depends on: 648273
Depends on: 652378
Depends on: 657719
Depends on: 671926
Depends on: 671991
Depends on: 673689
Assignee: nobody → trev.saunders
Whiteboard: bk1
Depends on: 717683
Depends on: 717689
Depends on: 726287
Depends on: 733335
Depends on: 739568
Depends on: 739883
Depends on: 739882
Depends on: 739884
Depends on: 739885
Depends on: 740375
Depends on: 740702
Depends on: 740725
Depends on: 740747
Depends on: 740758
Depends on: 740766
Depends on: 741697
Depends on: 741702
Depends on: 741703
Depends on: 741707
Depends on: 745429
No longer depends on: 740747
Depends on: 747219
Depends on: 747227
Depends on: 748639
Depends on: 757503
Depends on: 757504
Depends on: 760878
Depends on: 760880
Depends on: 760881
Depends on: 762038
Depends on: 765371
Depends on: 765512
Depends on: 767705
Depends on: 768461
Depends on: 777117
Depends on: 787308
Depends on: 795192
Depends on: 799909
Depends on: 829382
Depends on: 932789
Depends on: 934039
Depends on: 935698
Depends on: 1064877
Depends on: 1068734
Depends on: 1076816
we've done with DeXPOMination, next step will be moving to WebIDL but that's different story
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.