Closed Bug 917563 Opened 11 years ago Closed 10 years ago

Convert abSelectAddressesDialog.xul from a <window class="dialog"> to a <dialog>

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Product:
SeaMonkey
Component:
MailNews: Address Book & Contacts
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(seamonkey2.33 fixed)

Status:
RESOLVED FIXED
Milestone:
seamonkey2.33
Tracking Flags:
Tracking Status
seamonkey2.33 --- fixed

People

(Reporter: iannbugzilla, Assigned: philip.chee)

References

(Blocks 2 open bugs,
URL
)

Details

Attachments

(2 files, 1 obsolete file)

Patch v2 fix nits r=IanN
6.58 KB, patch
mnyromyr
: superreview+
Details | Diff | Splinter Review
Followup patch to remove dialog class
1.16 KB, patch
iannbugzilla
: review+
iannbugzilla
: approval-comm-aurora+
iannbugzilla
: approval-comm-beta+
Details | Diff | Splinter Review 
abSelectAddressesDialog.xul makes use of the obsolete dialogOverlay.xul overlay, it should just be a matter of converting it from <window class="dialog"> to <dialog>
Component: MailNews: Composition → MailNews: Address Book & Contacts
abSelectAddressesDialog.xul:
11 <?xul-overlay href="chrome://communicator/content/utilityOverlay.xul"?>
...
35   <script type="application/javascript" src="chrome://global/content/globalOverlay.js"/>

So globalOverlay.js is invoked twice.
Attached patch Patch v1 fixit (obsolete) — — Splinter Review 
A shockingly simple patch.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #8501837 - Flags: superreview?(mnyromyr)
Attachment #8501837 - Flags: review?(iann_bugzilla)
(In reply to Philip Chee from comment #1)
> abSelectAddressesDialog.xul:
> 11 <?xul-overlay href="chrome://communicator/content/utilityOverlay.xul"?>
> ...
> 35   <script type="application/javascript"
> src="chrome://global/content/globalOverlay.js"/>
> 
> So globalOverlay.js is invoked twice.

Did you consider this?
Flags: needinfo?(philip.chee)
Comment on attachment 8501837 [details] [diff] [review]
Patch v1 fixit

>+++ b/suite/mailnews/addrbook/abSelectAddressesDialog.xul

>+        buttons="accept,cancel"
>+        ondialogaccept="return SelectAddressOKButton()"
Nit: I know other lines are missing the ; but please add it here.

>         onload="OnLoadSelectAddress()"
>         onunload="OnUnloadSelectAddress()">
>+      <vbox id="resultsBox" flex="4">
>+        <tree id="abResultsTree" flex="1" persist="height" />
Nit: remove the space before the /

r=me but consider the need-info
Attachment #8501837 - Flags: review?(iann_bugzilla) → review+
>> So globalOverlay.js is invoked twice.
> Did you consider this?
Yes. It appears that I was mistaken.
requesting moa? from Mnyromyr.

>>+        buttons="accept,cancel"
>>r+        ondialogaccept="return SelectAddressOKButton()"
> Nit: I know other lines are missing the ; but please add it here.
Fixed.

>>r         onload="OnLoadSelectAddress()"
>>r         onunload="OnUnloadSelectAddress()">
>>r+      <vbox id="resultsBox" flex="4">
>>r+        <tree id="abResultsTree" flex="1" persist="height" />
> Nit: remove the space before the /
Fixed.
Attachment #8501837 - Attachment is obsolete: true
Attachment #8501837 - Flags: superreview?(mnyromyr)
Attachment #8502348 - Flags: superreview?(mnyromyr)
Flags: needinfo?(philip.chee)
Attachment #8502348 - Flags: superreview?(mnyromyr) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-seamonkey2.33: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.33
Comment on attachment 8502348 [details] [diff] [review]
Patch v2 fix nits r=IanN

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

::: suite/mailnews/addrbook/abSelectAddressesDialog.xul
@@ +20,3 @@
>          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>          title="&selectAddressWindow.title;"
>          class="dialog"

Did you miss to remove this "class" line? (Or is it still useful?)
Attachment #8502348 - Flags: feedback?(philip.chee)
(In reply to Serge Gautherie (:sgautherie) from comment #8)
> >          class="dialog"
> 
> Did you miss to remove this "class" line? (Or is it still useful?)
Yes thanks! I forgot to remove that line.

p.s. Please use NEEDINFO instead of feedback? for this sort of question.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(philip.chee)
Flags: needinfo?(philip.chee)
Attachment #8502348 - Flags: feedback?(philip.chee)
Status: REOPENED → NEW
Flags: needinfo?(philip.chee)
Closing to take this off my bug radar
Status: NEW → RESOLVED
Closed: 10 years ago → 10 years ago
Flags: needinfo?(philip.chee)
Resolution: --- → FIXED
(In reply to Philip Chee from comment #9)
> (In reply to Serge Gautherie (:sgautherie) from comment #8)
> > Did you miss to remove this "class" line? (Or is it still useful?)
> Yes thanks! I forgot to remove that line.

Could you remove the leftover line? Thanks.
Flags: needinfo?(philip.chee)
Flags: needinfo?(philip.chee)

        Attachment #8554583 -
        Flags: review?(iann_bugzilla)

    
    

    
  
Comment on attachment 8554583 [details] [diff] [review]
Followup patch to remove dialog class

[Triage Comment]
a=me for c-a/c-b

        Attachment #8554583 -
        Flags: review?(iann_bugzilla)

        Attachment #8554583 -
        Flags: review+

        Attachment #8554583 -
        Flags: approval-comm-beta+

        Attachment #8554583 -
        Flags: approval-comm-aurora+

    
    

    
  
(In reply to Ian Neal from comment #13)
> Comment on attachment 8554583 [details] [diff] [review]
> Followup patch to remove dialog class

http://hg.mozilla.org/comm-central/rev/663ea88077c3

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: