197209 - <folderProps.xul>: Convert <window class="dialog"> to <dialog>

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Sebastian Späth]

OK, timeless pointed me to this:

the mailnews folder properties dialog is still using <windows... instead of
<dialog...

This leads to some bad side effects, e.g. the ok and cancel buttons are reversed
when you do the "sidebar on the right side" customization thing
(http://mozilla.org/unix/customizing.html)

Patch follows shortly...
This will be my very first mozilla patch (indeed my very first patch at all) so
bear with me

Comment 1

[Sebastian Späth]

Attachment: patch

OK, I would be glad if somebody could have a look at the patch and tell me if I
forgot to take something out (unneccessary overlays, etc...).

I compiled on Linux and made sure that ok and cancel buttons work.

Comment 2

[Sebastian Späth]

The original file is at
http://lxr.mozilla.org/seamonkey/source/mailnews/base/resources/content/folderProps.xul

The <dialog> does not have an id="...". Would that be needed?
Thanks for reviewing

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

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

 <!DOCTYPE window SYSTEM "chrome://messenger/locale/folderProps.dtd">

this line needs a change as well

Comment 4

[Sebastian Späth]

Attachment: v.02

Changed the doctype as well. Thanks for the hint biesi.
Patch should be ready for review and checkin now...

Comment 5

[[e-mail]]

Comment on attachment 117184 [details] [diff] [review]
v.02

You should remove dialogOverlay and fix any dependency issues. You don't need
ondialogcancel for a start.

Comment 6

[Sebastian Späth]

OK, removed dialogOverlay dependency and oncanceldialog. Can provide a new patch...

However, I noticed an uncaught exception which already ocurred in the old code.
What is wrong with this?

This line:
var imapFolder =
    gMsgFolder.QueryInterface(Components.interfaces.nsIMsgImapMailFolder);

leads to:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002
(NS_NOINTERFACE)"  location: "JS frame ::
chrome://messenger/content/folderProps.js :: hideShowControls :: line 229" 
data: no]

I need halp on this one...

Comment 7

[Sebastian Späth]

Attachment: New patch, resolving Neils issues. The uncaught exception still needs to be resolved

Comment 8

[Sebastian Späth]

Comment on attachment 117311 [details] [diff] [review]
New patch, resolving Neils issues. The uncaught exception still needs to be resolved

I decided to spin the exception out to another bug, because that is really not
part of the xul cleanup I'm doing. See bug 197660 for this.
Neil, could I have a new review on the xul cleanup? Thanks

Comment 9

[Sebastian Späth]

Attachment: Even more cleanup (removed unneccessary dialog keyset)

Comment 10

[[e-mail]]

Comment on attachment 117392 [details] [diff] [review]
Even more cleanup (removed unneccessary dialog keyset)

Looks OK but I won't get to test it just yet.

Comment 11

[[e-mail]]

Comment on attachment 117392 [details] [diff] [review]
Even more cleanup (removed unneccessary dialog keyset)

Bug 197660 bitrotted this patch slightly but I've retested it and it's fine.

Comment 12

[[e-mail]]

Comment on attachment 117311 [details] [diff] [review]
New patch, resolving Neils issues. The uncaught exception still needs to be resolved

Cancelling review on obsolete patch.

Comment 13

[Stefan Borggraefe]

Comment on attachment 117392 [details] [diff] [review]
Even more cleanup (removed unneccessary dialog keyset)

Obsoleting patch since it bitrotted again.

Comment 14

[Stefan Borggraefe]

Attachment: Updated patch

Comment 15

[Stefan Borggraefe]

Comment on attachment 131745 [details] [diff] [review]
Updated patch

Transferred Neil's r+ since only the line numbers have changed. I tested the
patch with a current trunk build.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in mailnews/base/resources/content/folderProps.js;
/cvsroot/mozilla/mailnews/base/resources/content/folderProps.js,v  <--  folderPr
ops.js
new revision: 1.19; previous revision: 1.18
done
Checking in mailnews/base/resources/content/folderProps.xul;
/cvsroot/mozilla/mailnews/base/resources/content/folderProps.xul,v  <--  folderP
rops.xul
new revision: 1.14; previous revision: 1.13
done