1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog using gSubdialog

Status: RESOLVED FIXED

(Toolkit :: Form Manager, defect, P1)

Description

[Scott Wu [:scottwu]]

Once gSubdialog can handle opening multiple dialogs at once, the edit profiles dialog should open on top of the manage profiles dialog, rather than as a modal.

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

Review commit: https://reviewboard.mozilla.org/r/125302/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125302/

Comment 2

[Scott Wu [:scottwu]]

Comment on attachment 8853235 [details]
Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125302/diff/1-2/

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8853235 [details]
Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

https://reviewboard.mozilla.org/r/125302/#review151302

::: browser/extensions/formautofill/content/manageProfiles.js:18
(Diff revision 2)
>  
>  this.log = null;
>  FormAutofillUtils.defineLazyLogGetter(this, "manageProfiles");
>  
>  function ManageProfileDialog() {
> +  this.prefWin = window.arguments && window.arguments[0];

I suspect you could use `window.opener` instead of passing this through arguments but I could be wrong

Comment 4

[Scott Wu [:scottwu]]

Comment on attachment 8853235 [details]
Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

https://reviewboard.mozilla.org/r/125302/#review151302

> I suspect you could use `window.opener` instead of passing this through arguments but I could be wrong

I'm not sure how `window.opener` could do here. From what I can tell it shows the location of the manageProfiles dialog, but it doesn't have the profile information. An alternative which we discussed about was passing the profile data as params in URL. Would you think it's cleaner this way?

Comment 5

[Scott Wu [:scottwu]]

Comment on attachment 8853235 [details]
Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

https://reviewboard.mozilla.org/r/125302/#review151302

> I'm not sure how `window.opener` could do here. From what I can tell it shows the location of the manageProfiles dialog, but it doesn't have the profile information. An alternative which we discussed about was passing the profile data as params in URL. Would you think it's cleaner this way?

Sorry I misread your comment. `window.opener` does work here and I've revised the patch.

Comment 6

[Scott Wu [:scottwu]]

Comment on attachment 8853235 [details]
Bug 1352307 - [Form Autofill] Open the edit profile dialog on top of the manage profiles dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125302/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29f34e4a8b1a
[Form Autofill] Open the edit profile dialog on top of the manage profiles dialog. r=MattN

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/29f34e4a8b1a