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

VERIFIED FIXED in seamonkey2.35

Status

SeaMonkey
General
--
minor
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: sgautherie, Assigned: ewong)

Tracking

(Blocks: 2 bugs)

Trunk
seamonkey2.35
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
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"
}

Updated

4 years ago
Component: Page Info → General
Summary: Replace use of obsolete dialogOverlay.xul, in SeaMonkey → Replace remaining use of obsolete dialogOverlay.xul, in SeaMonkey (/suite/browser/metadata.xul)
(Assignee)

Comment 1

4 years ago
Created attachment 8550630 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8550630 - Flags: review?(neil)
(Reporter)

Comment 2

4 years ago
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?
Attachment #8550630 - Flags: feedback?(ewong)

Comment 3

4 years ago
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.
Attachment #8550630 - Flags: review?(neil) → review-
(Assignee)

Comment 4

4 years ago
(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.
(Assignee)

Comment 5

4 years ago
Created attachment 8551005 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul. (v2)
Attachment #8550630 - Attachment is obsolete: true
Attachment #8550630 - Flags: feedback?(ewong)
Attachment #8551005 - Flags: review?(neil)

Comment 6

4 years ago
> 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

4 years ago
(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?
(Assignee)

Comment 8

4 years ago
(In reply to neil@parkwaycc.co.uk 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.
(Assignee)

Comment 9

4 years ago
Created attachment 8552805 [details] [diff] [review]
Replace remaining use of obsolete dialogOverlay.xul. (v3)
Attachment #8551005 - Attachment is obsolete: true
Attachment #8551005 - Flags: review?(neil)
Attachment #8552805 - Flags: review?(neil)

Updated

4 years ago
Attachment #8552805 - Flags: review?(neil) → review+
(Assignee)

Comment 10

4 years ago
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
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.35
(Reporter)

Comment 11

4 years ago
V.Fixed, per MXR search.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.