Reset master password confirmation dialog should be opened as a dialog.

RESOLVED FIXED

Status

Core Graveyard
Security: UI
RESOLVED FIXED
12 years ago
2 years ago

People

(Reporter: Dorian, Assigned: kaie)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.83 KB, patch
kaie
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
Having a minimize box for a confirmation dialog is not that great.
(Reporter)

Comment 1

12 years ago
Created attachment 228119 [details] [diff] [review]
Patch1

Switching to openDialog
Attachment #228119 - Flags: review?(kengert)
(Assignee)

Comment 2

12 years ago
Comment on attachment 228119 [details] [diff] [review]
Patch1

onLoad in resetpassword.js uses this test

  if ("arguments" in window) 

Does that still work?
(Reporter)

Comment 3

12 years ago
Created attachment 228169 [details] [diff] [review]
Patch2

the if ("arguments" in window) is correct, but there was a problem. nsIDialogParamBlock wasn't declared, same problem in password.js
Attachment #228119 - Attachment is obsolete: true
Attachment #228119 - Flags: review?(kengert)
(Assignee)

Comment 4

12 years ago
Comment on attachment 228169 [details] [diff] [review]
Patch2

r=kengert
Attachment #228169 - Flags: review+

Comment 5

12 years ago
Does security/ require sr? If not, someone should check this in...
(Assignee)

Comment 6

12 years ago
thanks for making me aware, I'll check it in shortly
(Assignee)

Comment 7

12 years ago
fixed on trunk, thanks a lot for the patch
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

12 years ago
This caused regression bug 369098.

When I reviewed your patch, I failed to catch that you are introducing a duplicate declaration in file password.js
Removing that line fixes it for me.

I'll back out that line, it's an obvious mistake.
Blocks: 369098
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.