Closed Bug 658968 Opened 13 years ago Closed 13 years ago

javascript ternary operator evaluation bug

Categories

(Core :: JavaScript Engine, defect)

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

Tracking

()

Status:
VERIFIED FIXED
Tracking Flags:
Tracking Status
firefox5 --- fixed
firefox6 + fixed

People

(Reporter: stulraj, Assigned: jandem)

References

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(3 files, 1 obsolete file)

test script
1.68 KB, text/html
Details
screen output
66.82 KB, application/octet-stream
Details
Patch
2.37 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

Result of expression is allways '1' ('element' is checkbox):

tarr[tidx] = element.checked == true ? '1' : '0';

however, if-else construction works well:

if(element.checked == true)   
  tarr[tidx] = '1';
else  
  tarr[tidx] = '0';

Firefox/3.6.3 works with both constructions well.


Reproducible: Always
Can you write a complete minimal testcase? I tried this with:

<html>
<body>

<input type="checkbox"id="foo">

<script type="text/javascript">
var bar = document.getElementById('foo').checked == true ? '1' : '0';
alert(bar)
</script>

</body>
</html>

and it works correctly.

This is not a security-sensitive issue, so please don't abuse that flag.
Assignee: nobody → general
Group: core-security
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Heck, a testcase of any sort, even nonminimal, would be useful.  I also can't reproduce this problem.
Closing.  ms, please reopen if you can post a test-case that shows the problem.  Thanks!
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
Attached file test script — 

    
    

    
  

      
      
      
      

        Attached file
          
        screen output
        — 
      

    


  

    
    

    
  
Testcase and screen outputs added. Change of any checkbox outputs current status of all checkboxes. Each additional change shows incorect checkboxes status.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---

    
  

        Attachment #534695 -
        Attachment mime type: text/plain → text/html

    
    

    
  
Thanks ms! This is a bug in JM, introduced in FF4.
Assignee: general → jandemooij
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch (obsolete)
        — 
          — Splinter Review
      

    


  
The syncAndForgetEverything in emitStubCmp could clobber Registers::ReturnReg. This also fixes a similar problem in jsop_relational_double.

The TI-branch does not have this problem because it does not sync between the stub call and the branch. Do we want to wait for the TI merge or land this in the meantime? Maybe for FF 6?

        Attachment #534736 -
        Flags: review?(dvander)

    
  
Blocks: Jaeger

    
    

    
  
This looks safe and important enough that I'd take it for FF5 beta if reviews are completed promptly.

          status-firefox5:
          --- → affected

          tracking-firefox6:
          --- → +
Keywords: regression

    
    

    
  
Comment on attachment 534736 [details] [diff] [review]
Patch

Nice catch.

        Attachment #534736 -
        Flags: review?(dvander)

        Attachment #534736 -
        Flags: review+

        Attachment #534736 -
        Flags: approval-mozilla-beta+

    
    

    
  
http://hg.mozilla.org/tracemonkey/rev/68bc1acb7f3e

I don't have level 3 commit access, so I'll need somebody to push this to mozilla-aurora and mozilla-beta (if it's green on the TM tinderbox).
Whiteboard: fixed-in-tracemonkey

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch
        — 
          — Splinter Review
      

    


  
This is the patch that landed on TM (I added "and jsop_relational_double" to the commit message).

        Attachment #534736 -
        Attachment is obsolete: true

        Attachment #534736 -
        Flags: approval-mozilla-aurora+

    
    

    
  
Mozilla/5.0 (Windows NT 6.1; rv:5.0) Gecko/20100101 Firefox/5.0

I verified this issue and it looks as being fixed, but the status of this bug is Assigned. Shouldn't the status be: Resolved Fixed?

    
    

    
  
It's waiting for a merge from the tracemonkey repository to the mainline mozilla-central repository before it's marked fixed.  The bug status refers to the status on the trunk, the branches have their own status-firefoxN flags (which are correctly set to fixed).

    
    

    
  
It looks like this is on m-c and has been for a while.  Is there a reason the bot didn't mark this fixed?

    
    

    
  
There were a few times I marked it by the tbpl log on the merge push and it seems like that misses some things.

http://hg.mozilla.org/mozilla-central/rev/68bc1acb7f3e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED

    
    

    
  
Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110711 Firefox/8.0a1

Verified issue using the test case from Comment 4 on Windows 7.

Setting resolution to VERIFIED FIXED.
Status: RESOLVED → VERIFIED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: