Fix issue where diffs fail to update when moving between revisions

 






 

 


  Review Request #6261
  —
  Created Aug. 24, 2014 and discarded 

 


















 

  

   Information

  





 
 

  guffin
 









 
 

  Review Board
 









 
 

  master
 









 
 

  3514
 









 
 

  
 









 
 

  1837d7d...
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Backbone.View's this.el is not getting initialized (perhaps intentionally, I cannot tell).  The fix is to test for validity before using it.

 








  


 
 

  
Checked that loading pages with multiple revisions can be changed with slider in the process of producing diffs.  Ran all unit tests.

 







 









 





  








 









 






  

 








 














 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Can you provide some explanation of what you think this change accomplishes and how it fixes the bug?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Hmm, I am apparently using rbt incorrectly.  This is not the diff I intended to upload.  I'll try again with one from a github fork.
      

      The problem is that Backbone.View's this.el is not getting initialized (perhaps intentionally, I cannot tell).  The fix is to test for validity before using it.
      
 
      

      2. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   





 GU





   

   

  

 



 















 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I don't understand the case where this happens. I haven't seen it myself.
    

    I'd need to see solid repro steps that I can trigger, and we should probably get a JavaScript unit test that demonstrates the behavior and shows it's fixed.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Complete reproduction steps are available on the bug description.  It is visible on this site (reviews.reviewboard.org).  Reproduced here from the bug:
      

      https://reviews.reviewboard.org/r/5869/diff
      

        

      1. Scroll down to the diff slider controls
        2. 

      2. Move left the slider to position 1 (load diff 1-14)
        3. 

      3. Before all files can complete loading (ajax spinners still show), move the slider to position 2 (load diff/2-14)
        4. 

      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      2 should read 'Move the left slider...'
      
 
      

      3. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I'm going to fix this bug in a slightly different way (cancelling the diff view at a higher level rather than just fixing up the invalid accesses).
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   





 GU





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Discarded

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...