Closed Bug 1352307 Opened 3 years ago Closed 2 years ago

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

Categories

(Toolkit :: Form Manager, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: scottwu, Assigned: scottwu)

References

Details

(Whiteboard: [form autofill:M3] ETA:612)

Attachments

(1 file)

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.
Component: Preferences → Form Manager
Product: Firefox → Toolkit
Whiteboard: [form autofill:M1] → [form autofill:M2]
Whiteboard: [form autofill:M2] → [form autofill:M2] ETA:612
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
Attachment #8853235 - Flags: review?(MattN+bmo) → review+
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 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.
Keywords: checkin-needed
Whiteboard: [form autofill:M2] ETA:612 → [form autofill:M3] ETA:612
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/29f34e4a8b1a
[Form Autofill] Open the edit profile dialog on top of the manage profiles dialog. r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/29f34e4a8b1a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.