Closed Bug 280136 Opened 15 years ago Closed 11 years ago

Remove GetFinalState()

Categories

(Core :: Disability Access APIs, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(2 files, 1 obsolete file)

The methods nsIAccessible::GetFinalRole(), GetFinalState() and GetFinalValue()
were added to make the DHTML accessibily patch in bug 249998 small and reviewable.

Changing the code to remove the need for methods is going to be a large patch,
but it should be done. It's ugly to have GetRole and GetFinalRole, etc.
Blocks: aria
Blocks: cleana11y
No longer blocks: aria
I think we might want to have some methods with names like GetStateInternal(), which are virtual but not interface methods. That's where the non-ARIA work would be done. But, it would mean there are still 2 methods for role, 2 methods for state, etc. However, I don't want a situation where every new impl class for a11y needs to remember to call methods for ARIA work.

Let's safe this for after Firefox 3.
Summary: Remove nsIAccessible::GetFinalRole(), GetFinalState() and GetFinalValue() → Remove GetFinalState()
Attached patch inspector-patch (obsolete) — Splinter Review
Assignee: aaronleventhal → surkov.alexander
Status: NEW → ASSIGNED
Attachment #345842 - Flags: superreview?(neil)
Attachment #345842 - Flags: review?(sdwilsh)
Attached patch mozilla-patchSplinter Review
Attachment #345842 - Attachment is obsolete: true
Attachment #345843 - Flags: review?(aaronleventhal)
Attachment #345842 - Flags: superreview?(neil)
Attachment #345842 - Flags: review?(sdwilsh)
Comment on attachment 345842 [details] [diff] [review]
inspector-patch

sorry, it was excess mouse click
Attachment #345842 - Attachment is obsolete: false
Attachment #345842 - Flags: superreview?(neil)
Attachment #345842 - Flags: review?(sdwilsh)
Will you need to change the minVersion in DOMi's install.rdf?
For the inspector patch, we'll still need to be able to support 3.0, so can you have this degrade gracefully?
Attached patch inspector-patch2Splinter Review
Does it work for you?
Attachment #345842 - Attachment is obsolete: true
Attachment #345915 - Flags: superreview?(neil)
Attachment #345915 - Flags: review?(sdwilsh)
Attachment #345842 - Flags: superreview?(neil)
Attachment #345842 - Flags: review?(sdwilsh)
Attachment #345915 - Flags: superreview?(neil) → superreview+
Comment on attachment 345843 [details] [diff] [review]
mozilla-patch

David, how would you like a nice code reorg review? :)
Attachment #345843 - Flags: review?(aaronleventhal) → review?(david.bolter)
Sure. Can it wait until tonight?

(In reply to comment #8)
> (From update of attachment 345843 [details] [diff] [review])
> David, how would you like a nice code reorg review? :)
Comment on attachment 345843 [details] [diff] [review]
mozilla-patch

You have my r+ :)

Aaron owes me a doughnut.

Wow that was tedious, but this makes the code much nicer. Surkov, nice catch fixing the mochitests too. I'm loving the NS_IMETHOD to "virtual nsresult", and NS_IMETHODIMP to "nsresult" changes. Starting to look like C++ again :)
Attachment #345843 - Flags: review?(david.bolter) → review+
Comment on attachment 345915 [details] [diff] [review]
inspector-patch2

r=sdwilsh
Attachment #345915 - Flags: review?(sdwilsh) → review+
Blocks: 457254
inspector's changeset: http://hg.mozilla.org/dom-inspector/rev/f70f22b6a96e
mozilla's changeset: http://hg.mozilla.org/mozilla-central/rev/ed2e583d9edf
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.