1106536 - Replace remaining use of obsolete dialogOverlay.xul, in SeaMonkey (/suite/browser/metadata.xul)

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

Ftr,
http://mxr.mozilla.org/mozilla-central/source/toolkit/obsolete/content/dialogOverlay.xul

Last use in /suite:
{
/suite/browser/metadata.xul
    * line 10 -- <?xul-overlay href="chrome://global/content/dialogOverlay.xul"?>
}

And, wrt bug 226958:
{
10 <?xul-overlay href="chrome://global/content/dialogOverlay.xul"?>
11 
12 <!DOCTYPE window [
...
17 <window id="metadata"
...
21         class="dialog"
}

Comment 1

[Edmund Wong (:ewong)]

Attachment: Replace remaining use of obsolete dialogOverlay.xul.

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 8550630 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul.

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

::: suite/browser/metadata.xul
@@ +16,4 @@
>          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>          title="&caption.label;"
>          onload="onLoad()"
>          class="dialog"

Is this 'class' attribute still needed?

Comment 3

[[e-mail]]

Comment on attachment 8550630 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul.

>+<dialog id="metadata"
XML Parsing Error: mismatched tag. Expected: </dialog>.
Location: chrome://navigator/content/metadata.xul
Line Number 194, Column 4:

 </window>
---^

>         class="dialog"
(As Serge says, this is obsolete.)

You didn't remove the keyset.

You need to remove the default dialog buttons.

Comment 4

[Edmund Wong (:ewong)]

(In reply to Serge Gautherie (:sgautherie) from comment #2)
> Comment on attachment 8550630 [details] [diff] [review]
> Replace remaining use of obsolete dialogOverlay.xul.
> 
> Review of attachment 8550630 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: suite/browser/metadata.xul
> @@ +16,4 @@
> >          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
> >          title="&caption.label;"
> >          onload="onLoad()"
> >          class="dialog"
> 
> Is this 'class' attribute still needed?

No.  It's not needed.

Comment 5

[Edmund Wong (:ewong)]

Attachment: Replace remaining use of obsolete dialogOverlay.xul. (v2)

Comment 6

[Philip Chee]

> You didn't remove the keyset.
> You need to remove the default dialog buttons.
Alternative proposal:
1. remove the dialogOverlay.xul but keep metadata.xul a window.
2. add the following keys from globalOverlay.xul to the metadata.xul keyset:
2a. "VK_ESCAPE" and 
2b. |key="." modifiers="meta"|
See http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/obsolete/content/dialogOverlay.xul?rev=e947cee33a92&mark=94-97#92
3. replace |doCancelButton()| with |window.close();|

Comment 7

[[e-mail]]

(In reply to Edmund Wong from comment #5)
> Created attachment 8551005 [details] [diff] [review]
> Replace remaining use of obsolete dialogOverlay.xul. (v2)
Buttons are still there.

(In reply to Philip Chee from comment #6)
> Alternative proposal:
> 1. remove the dialogOverlay.xul but keep metadata.xul a window.
What about the CSS?

Comment 8

[Edmund Wong (:ewong)]

(In reply to [e-mail] from comment #7)
> (In reply to Edmund Wong from comment #5)
> > Created attachment 8551005 [details] [diff] [review]
> > Replace remaining use of obsolete dialogOverlay.xul. (v2)
> Buttons are still there.


I have the patch that removes the buttons; but doesn't it make
sense to have at least "OK" there?  I find it weird to just have
a dialog without any buttons or am I missing something?  Prior to
this patch, the buttons weren't there and the user had to click on
the X on the window.

Comment 9

[Edmund Wong (:ewong)]

Attachment: Replace remaining use of obsolete dialogOverlay.xul. (v3)

Comment 10

[Edmund Wong (:ewong)]

Comment on attachment 8552805 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul. (v3)

Pushed to comm-central:
 https://hg.mozilla.org/comm-central/rev/e1868ba1d4dc

Comment 11

[Serge Gautherie (:sgautherie)]

V.Fixed, per MXR search.