Closed Bug 351687 Opened 18 years ago Closed 16 years ago

[@ nsBlockFrame::GetAccessible] mContent null check comes after we use mContent

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

kill it
809 bytes, patch
aaronlev
: review+
dbaron
: superreview+
beltzner
: approval1.9+
Details | Diff | Splinter Review 
 
It looks like I checked mContent for null after it was already used. It should be checked before.
Blocks: keya11y
Severity: normal → critical
Summary: nsBlockFrame::GetAccessible(nsIAccessible **) has a hopefully useless null check of mContent → [@ nsBlockFrame::GetAccessible] crashes dereferencing mContent before null check
Attached patch reorder (obsolete) — — Splinter Review 
Assignee: aaronleventhal → timeless
Status: NEW → ASSIGNED

        Attachment #238880 -
        Flags: review?(aaronleventhal)

    
  

        Attachment #238880 -
        Flags: review?(aaronleventhal) → review+

    
    

    
  
Why wasn't this checked in?

    
    

    
  
Comment on attachment 238880 [details] [diff] [review]
reorder

because we require sr's and i don't track bugs so someone has to shepherd them through the process...

        Attachment #238880 -
        Flags: superreview?(dbaron)

    
    

    
  
Has anyone actually seen this crash or is this just theoretical?  If the former, why was mContent null?  If the latter, do you have reason to believe that mContent is sometimes null?

    
    

    
  
Comment on attachment 238880 [details] [diff] [review]
reorder

I think it's the null-check itself that's bogus; show me a block frame creation path that can happen with a null content node and I'll consider it.

        Attachment #238880 -
        Flags: superreview?(dbaron) → superreview-

    
  
Blocks: fox3access

    
    

    
  
Dbaron, I've never seen this crash. But, we should either have the proper null check, change it to an assertion or remove it entirely.

    
  
No longer blocks: fox3access

    
  
Summary: [@ nsBlockFrame::GetAccessible] crashes dereferencing mContent before null check → [@ nsBlockFrame::GetAccessible] mContent null check comes after we use mContent

    
    

    
  
What's the status on this? Aaron, do crash reports show any problems in this area? Or should this be marked WONTFIX if the NULL check is bogus anyway?

    
    

    
  
Marco, this is low priority. We should can the bogus check since it doesn't do anything.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        kill it
        — 
          — Splinter Review
      

    


  

        Attachment #238880 -
        Attachment is obsolete: true

        Attachment #313952 -
        Flags: review?(aaronleventhal)

    
  

        Attachment #313952 -
        Flags: review?(aaronleventhal) → review+

    
  

        Attachment #313952 -
        Flags: superreview?(dbaron)

    
    

    
  
Comment on attachment 313952 [details] [diff] [review]
kill it

sr=dbaron

        Attachment #313952 -
        Flags: superreview?(dbaron) → superreview+

    
    

    
  
Comment on attachment 313952 [details] [diff] [review]
kill it

a1.9=beltzner

        Attachment #313952 -
        Flags: approval1.9+

    
    

    
  
Comment on attachment 313952 [details] [diff] [review]
kill it

mozilla/layout/generic/nsBlockFrame.cpp 	3.951
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: