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

RESOLVED FIXED in seamonkey2.33

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: iann_bugzilla, Assigned: philip.chee)

Tracking

(Blocks 2 bugs)

Trunk
seamonkey2.33
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.33 fixed)

Details

()

Attachments

(2 attachments, 1 obsolete attachment)

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.
Posted 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: 5 years ago
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: 5 years ago5 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.