Open Bug 343586 Opened 18 years ago Updated 2 years ago

should preferences.xml and scrollbox.xml check the document in notify()?

Categories

(Toolkit :: UI Widgets, defect)

Product:
Toolkit
Component:
UI Widgets
Platform:
x86
Windows XP
Type:
defect

Tracking

()

People

(Reporter: moco, Unassigned)

References

Details

Attachments

(1 file)

remove it on trunk
1.84 KB, patch
Gavin
: first-review+
Details | Diff | Splinter Review 
should preferences.xml and scrollbox.xml check the document in notify()?

here's the change in question:

>+      <method name="notify">
>+        <parameter name="aTimer"/>
>+        <body>
>+        <![CDATA[
>+          if (!document)
>+            aTimer.cancel();
>+

asaf writes:

This doesn't make sense to me, but as you noted this is also done in the
preferences bindings (with "new options dialog" blame comment). Keep this here
but please file a bug on this (for both bindings).
Assignee: nobody → mano
Status: NEW → ASSIGNED

        Attachment #247587 -
        Flags: first-review?(gavin.sharp)

    
    

    
  
Comment on attachment 247587 [details] [diff] [review]
remove it on trunk

>Index: toolkit/content/widgets/preferences.xml

>+        <![CDATA[          

nit: trailing whitespace

I wish I knew why this was added in the first place... r=me if you test to make that increasing the timeout and closing the window while the timer is active doesn't cause trouble.

        Attachment #247587 -
        Flags: first-review?(gavin.sharp) → first-review+
Assignee: asaf → nobody
Status: ASSIGNED → NEW

    
  
Severity: normal → S3

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: