Closed Bug 1343385 Opened 7 years ago Closed 6 years ago

simplify selection detection in SMTP list editor in Account manager

Categories

(MailNews Core :: Account Manager, defect)

Product:
MailNews Core
Component:
Account Manager
Type:
defect
Priority:
Not set
Severity:
trivial

Tracking

(thunderbird60 fixed, thunderbird61 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 61.0
Tracking Flags:
Tracking Status
thunderbird60 --- fixed
thunderbird61 --- fixed

People

(Reporter: aceman, Assigned: aceman)

Details

Attachments

(1 file, 1 obsolete file)

1343385.patch v1.1
5.47 KB, patch
aceman
: review+
Details | Diff | Splinter Review 
Cleanup selection detection in SMTP list editor in Account manager. It seems to me some code can be simplified in am-smtp.js.
Attached patch patch (obsolete) — — Splinter Review 

        Attachment #8842209 -
        Flags: review?(mkmelin+mozilla)

    
    

    
  
Comment on attachment 8842209 [details] [diff] [review]
patch

Can we move this simple cleanup? :)

        Attachment #8842209 -
        Flags: review?(mkmelin+mozilla) → review?(jorgk)

    
    

    
  
Comment on attachment 8842209 [details] [diff] [review]
patch

Review of attachment 8842209 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't run it but the changes look plausible. Thanks for the clean-up.

::: mailnews/base/prefs/content/am-smtp.js
@@ +50,1 @@
>        // confirm deletion

Please fix indentation of this block.

@@ +253,5 @@
>    getSelectedServer: function()
>    {
> +    // The list of servers is a single selection listbox
> +    // therefore 1 item is always selected.
> +    // But when there are no smtp servers defined yet, nothing will be selected.

Unless there's a good reason not to, I'd like to move to all upper acronyms, like SMTP.

        Attachment #8842209 -
        Flags: review?(jorgk) → review+

    
    

    
  


  
Done, thanks.

        Attachment #8842209 -
        Attachment is obsolete: true

        Attachment #8958165 -
        Flags: review+

    
  
Keywords: checkin-needed

    
    

    
  
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/60c20fc98df1
consolidate selection checks in am-smtp.js. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

          status-thunderbird61:
          --- → fixed
Target Milestone: --- → Thunderbird 61.0

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: