Closed Bug 917563 Opened 12 years ago Closed 11 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 1 open bug,
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: 11 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: 11 years ago → 11 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: