Closed Bug 403794 Opened 16 years ago Closed 16 years ago

Crashes and unit'd aExtraState in various ::GetState() impls

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: aaronlev)

References

Details

(Keywords: access, crash, regression)

Attachments

(1 file, 1 obsolete file)

GetState() cleanup
38.00 KB, patch
surkov
: review+
damons
: approval1.9+
Details | Diff | Splinter Review 
We used to return an error for nsAccessible::GetState() when an object had been shut down. We don't do that anymore, but we forgot to add new checks mDOMNode in the various GetState() methods. This can cause crashes.
Blocks: fox3access
Attached patch GetState() cleanup (obsolete) — — Splinter Review 

        Attachment #288710 -
        Flags: review?(surkov.alexander)

    
    

    
  


  
Also, we most classes don't need to do anything special for STATE_FOCUSABLE anymore since nsAccessible impl does that with frame->IsFocusable() now.

        Attachment #288713 -
        Flags: review?(surkov.alexander)

    
  

        Attachment #288710 -
        Attachment is obsolete: true

        Attachment #288710 -
        Flags: review?(surkov.alexander)

    
    

    
  
Marco, if you have time to try running with this patch that'd be great.

    
    

    
  
Marco, if you have time to try running with this patch that'd be great.

    
    

    
  
1. nsXULToolbarSeparatorAccessible::GetState

+*aExtraState = mDOMNode ? nsIAccessibleStates::EXT_STATE_DEFUNCT : 0;.

Get back check on aExtraState.


2. nsXULListitemAccessible::GetState(

if (mIsCheckbox) {
  return nsXULMenuitemAccessible::GetState(aState, aExtraState);

Up this line otherwise you will set EXT_STATE_DEFUNCT twice

the rest looks ok

    
    

    
  
Comment on attachment 288713 [details] [diff] [review]
GetState() cleanup

r=me with comment above

        Attachment #288713 -
        Flags: review?(surkov.alexander) → review+

    
    

    
  
Comment on attachment 288713 [details] [diff] [review]
GetState() cleanup

Will address Surkov's comments at checkin.

        Attachment #288713 -
        Flags: approval1.9?

        Attachment #288713 -
        Flags: approval1.9? → approval1.9+

    
  
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.