Closed Bug 420966 Opened 15 years ago Closed 15 years ago

array_concat needs to deal with wrappers

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9beta5

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

Attachments

(1 file, 2 obsolete files)

Really updated
5.48 KB, patch
mrbkap
: review+
beltzner
: approval1.9+
Details | Diff | Splinter Review
Attached patch Fix (obsolete) — — Splinter Review 
array_concat only expands its argument if it is an array (as told by OBJ_IS_ARRAY). If the argument is a SJOW, then it won't expand it, even if the wrapped object *is* an array.

As wrappers should be transparent, we should expand the wrapped array anyway. I took this opportunity to define a js_GetWrappedObject and use it.
Attachment #307325 - Flags: review?(shaver)
Attachment #307325 - Flags: review?(brendan)
Comment on attachment 307325 [details] [diff] [review]
Fix

>+            if (aobj && OBJ_IS_ARRAY(cx, js_GetWrappedObject(cx, aobj))) {

Macro evals its second arg twice!

>+extern JSObject *
>+js_GetWrappedObject(JSContext *cx, JSObject *obj);

Comment infallibility.

Bonus for removing that blank line I mentioned in person ;-).

/be
Attachment #307325 - Flags: review?(brendan) → review+
Flags: blocking1.9?
Attached patch Updated to comments (obsolete) — — Splinter Review 
Assignee: general → mrbkap

        Attachment #307325 -
        Attachment is obsolete: true
Status: NEW → ASSIGNED

        Attachment #307325 -
        Flags: review?(shaver)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Really updated
        — 
          — Splinter Review
      

    


  
Sorry for the spam -- missed the extra newline!

        Attachment #307343 -
        Attachment is obsolete: true

        Attachment #307344 -
        Flags: review+

        Attachment #307344 -
        Flags: approval1.9?
Version: unspecified → Trunk

    
    

    
  
Comment on attachment 307344 [details] [diff] [review]
Really updated

a1.9=beltzner

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

    
    

    
  
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
Flags: in-testsuite-
Flags: in-litmus-

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