226955 - Bug 197315, </xpfe/*> files: Convert <window class="dialog"> to <dialog>

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

See bug 197315 "ToDoList" attachment:
there are +/- 12 <.xul> files to process.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <pageInfo.*>

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 136502 [details] [diff] [review]
(Av1) <pageInfo.*>


'r=?':
Can you review, test, check it in ?

Comment 3

[[e-mail]]

Comment on attachment 136502 [details] [diff] [review]
(Av1) <pageInfo.*>

You need buttons="help", the rest looks OK though.

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) <pageInfo.*>

Patch v1, plus comment 3 suggestions.

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Av2b) <pageInfo.*> [Checked in: Comment 21]

Comment 6

[[e-mail]]

IIRC db48x was doing stuff to page info, he might like to see the patch too.

Comment 7

[Daniel Brooks [:db48x]]

Comment on attachment 136899 [details] [diff] [review]
(Av2b) <pageInfo.*>
[Checked in: Comment 21]

yea, looks good

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 136899 [details] [diff] [review]
(Av2b) <pageInfo.*>
[Checked in: Comment 21]


'approval1.6=?': Trivial U.I. code cleanup.

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) <about.*>

I don't know how / can't remove the <about.js> file from the CVS repository...

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 137259 [details] [diff] [review]
(Bv1) <about.*>

Call: |window.openDialog("chrome://global/content/about.xul", "About",
"modal,chrome,resizable=yes,height=450,width=550");|

(related) Changed behaviours:
*Ctrl-PgUp/PgDn work immediately :-)
*Something (bottom of the image) inside <about:> now gets the initial visible
focus :-(


'r=?': (see comment 9)
Can you (super-)review, test, check it in ?

Comment 11

[[e-mail]]

Comment on attachment 137259 [details] [diff] [review]
(Bv1) <about.*>

>+    // Hidden preference, which can only be set manually by the user.
This is superfluous... the rest has r=me, although I don't like the +>.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 136899 [details] [diff] [review]
(Av2b) <pageInfo.*>
[Checked in: Comment 21]

please request approval *after* you've secured the necessary reviews

Comment 13

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1b) <about.*> [Checked in: Comment 19]

Bv1, with comment 11 suggestion(s),
plus 1 Tab->Spaces.

Comment 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 138662 [details] [diff] [review]
(Bv1b) <about.*>
[Checked in: Comment 19]


'r+' from Bv1.

Comment 15

[Alec Flett]

who's bv1? Who gave the r= here?

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 138662 [details] [diff] [review]
(Bv1b) <about.*>
[Checked in: Comment 19]


Sorry, I was not clear, I meant:
"(Bv1)	 patch	 2003-12-11 11:17:13   neil.parkwaycc.co.uk: review+"

Comment 17

[Alec Flett]

Comment on attachment 138662 [details] [diff] [review]
(Bv1b) <about.*>
[Checked in: Comment 19]

that's more like it.

assuming we don't need the id's for the frames,
sr=alecf

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 138662 [details] [diff] [review]
(Bv1b) <about.*>
[Checked in: Comment 19]


LXR search:
"aboutframe": unused.
"creditsframe": was used by <about.js>, "now" removed.

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 138662 [details] [diff] [review]
(Bv1b) <about.*>
[Checked in: Comment 19]


Check in: { 01/14/2004 02:37	neil%parkwaycc.co.uk }

Comment 20

[jag (Peter Annema)]

Comment on attachment 136899 [details] [diff] [review]
(Av2b) <pageInfo.*>
[Checked in: Comment 21]

sr=jag

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 136899 [details] [diff] [review]
(Av2b) <pageInfo.*>
[Checked in: Comment 21]


Check in: { 02/07/2004 05:08	neil%parkwaycc.co.uk }

Comment 22

[u60234]

Since this bug is still open I am putting this here instead of opening a new bug.

From your change to pageInfo.dtd:

>+<!ENTITY  copy.accesskey        "c">
>+<!ENTITY  closeWindow.accesskey "w">

Why are these command keys called accesskey? Could confuse a localizer.

Comment 23

[Serge Gautherie (:sgautherie)]

(In reply to comment #22)
> 
> From your change to pageInfo.dtd:
> 
> >+<!ENTITY  copy.accesskey        "c">
> >+<!ENTITY  closeWindow.accesskey "w">
> 
> Why are these command keys called accesskey? Could confuse a localizer.

I wanted to add an extension, guessed, got reviews...
I'm ready to prepare an additional patch:
What would be your hint ? .key, .keybinding, .commandkey, ...

Comment 24

[u60234]

Not sure which one is preferred here, they all look good to me. .key seems to be
more used than .commandkey, so if no one else object, use that one. 

Comment 25

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) <pageInfo.*> .accesskey/.key addition [Checked in: Comment 29]

Addition to Av2b, per comment 24 suggestion(s).

Comment 26

[jag (Peter Annema)]

Comment on attachment 141447 [details] [diff] [review]
(Cv1) <pageInfo.*> .accesskey/.key addition
[Checked in: Comment 29]

sr=jag

Comment 27

[Serge Gautherie (:sgautherie)]

Comment on attachment 141447 [details] [diff] [review]
(Cv1) <pageInfo.*> .accesskey/.key addition
[Checked in: Comment 29]


'approval1.7a=?': trivial UI 'naming' patch, "no" risk.

Comment 28

[Andreas Kunz]

Comment on attachment 141447 [details] [diff] [review]
(Cv1) <pageInfo.*> .accesskey/.key addition
[Checked in: Comment 29]

Clearing obsolete approval request (1.7a is tagged and fix is in).

Comment 29

[Serge Gautherie (:sgautherie)]

Comment on attachment 141447 [details] [diff] [review]
(Cv1) <pageInfo.*> .accesskey/.key addition
[Checked in: Comment 29]


Check in: { 02/19/2004 03:33	neil%parkwaycc.co.uk }

Comment 30

[[e-mail]]

Note that following the bug 78274 landing there are extra possibilities for
cleanup. Examples:
* customize.xul/js currently has code to handle its left-justified button, but
if you look at profileSelection.xul you'll see that you can now do that purely
in dialog attributes.
* p3pDialog.xul currently has code to label the buttons, which you can do do
purely using the right attributes.

Comment 31

[Stefan Borggraefe]

Attachment: conversion of pref-calibrate-screen.xul

Comment 32

[Stefan Borggraefe]

Bug 250609 blocks the conversion of printPreviewProgress.xul and metadata.xul.

Comment 33

[Serge Gautherie (:sgautherie)]

Comment on attachment 170981 [details] [diff] [review]
conversion of pref-calibrate-screen.xul

>- Contributor(s):
>+ Contributor(s): Stefan Borggraefe <[e-mail]>

nit: I think that you are supposed to add your name _under_ the title line.
(see some other files...)

Comment 34

[[e-mail]]

(In reply to comment #32)
>Bug 250609 blocks the conversion of printPreviewProgress.xul and metadata.xul.
They're not dialogs :-P

Comment 35

[[e-mail]]

Comment on attachment 170981 [details] [diff] [review]
conversion of pref-calibrate-screen.xul

I don't think we need Init() any more. I can see scope for further cleanup, for
instance by setting rv to { newdpi: -1 }; in changeScreenResolution().

Comment 36

[Stefan Borggraefe]

Attachment: conversion of pref-calibrate-screen.xul V1.1 [Checked in: Comment 41]

More clean-up like Neil suggested. I also picked up Serge's nit.

Comment 37

[Stefan Borggraefe]

(In reply to comment #34)
> (In reply to comment #32)
> >Bug 250609 blocks the conversion of printPreviewProgress.xul and metadata.xul.
> They're not dialogs :-P

So these can continue to use class="dialog" and dialogOverlay.xul or what should
happen to them? I thought we may be able to get rid of dialogOverlay.xul
altogether finally.

Comment 38

[Alec Flett]

Comment on attachment 171020 [details] [diff] [review]
conversion of pref-calibrate-screen.xul V1.1
[Checked in: Comment 41]

sorry for the delay - not sure if this request is still valid/necessary, but
sr=alecf just in case, this cleanup looks fine.

Comment 39

[Stefan Borggraefe]

Comment on attachment 171020 [details] [diff] [review]
conversion of pref-calibrate-screen.xul V1.1
[Checked in: Comment 41]

Requesting a= for seamonkey only, fairly low risk change.

Comment 40

[chris hofmann]

Comment on attachment 171020 [details] [diff] [review]
conversion of pref-calibrate-screen.xul V1.1
[Checked in: Comment 41]

a=chofmann

Comment 41

[Serge Gautherie (:sgautherie)]

Comment on attachment 171020 [details] [diff] [review]
conversion of pref-calibrate-screen.xul V1.1
[Checked in: Comment 41]


Check in: { 2005-05-04 19:25	Stefan.Borggraefe%gmx.de	mozilla/ xpfe/
components/ prefwindow/ resources/ content/ pref-fonts.js	 1.52 }

Comment 42

[Philip Chee]

/xpfe/ is DEAD. Closing bug.