1616179 - "Clear Recent History" dialog no longer remembers position

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Kestrel]

STR:

1. Press Ctrl+Shift+Del to show the "Clear Recent History" dialog.
2. Move the dialog and then close it.
3. Press Ctrl+Shift+Del again.
   (If you are on GNOME you will need to disable "Attach Modal Dialogs" in Tweaks)

Expected:

Dialog opens in previous moved location.

Actual:

Dialog opens in default location.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=88b25807638231dcf403d8f0b597fc770f1d24a4&tochange=e183cbb4983cfb3aecf97ab18fad916b91f89e7e

Regressed by Bug 1585482.

Comment 1

[Robin Steuber (she/her) [:bytesized]]

I'm afraid I am no longer working with the Browser Architecture team, and I don't have the time to work on this bug.

I did take a few minutes to look into it, and I can reproduce the problem. It's curious though, because I believe the way this value is persisted is through persist="screenX screenY", which correctly got migrated from the <dialog> element to the <window> element. I'm not really sure why it isn't working.

Comment 2

[Brian Grinstead [:bgrins]]

It appears that for some reason the persisted screenX and screenY attribute values aren't being restored the next time the window is opened. I think I know why - the window no longer has an [id] to key the value in XULStore (the id moved with the <dialog>): https://searchfox.org/mozilla-central/rev/df94cd5ba431234bc220ac081def0801fe44b89e/browser/base/content/sanitize.xhtml#24.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1616179 - Fix `JavaScript error: chrome://browser/content/sanitizeDialog.js, line 70: TypeError: right-hand side of 'in' should be an object, got null` error when sanitize dialog is opened outside of preferences;r=jaws

This condition was landed in Bug 1523696 and verified at that time, which leads me to think that
something may have changed with the opener in the meantime.

This is unrelated to the fix in the next patch, just something I noticed when debugging it.

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: Bug 1616179 - Assign an id to the <window> for the sanitize dialog to fix location persistence;r=jaws

The [persist] attribute requires the element has an id, so it was failing to remember
screenX and screenY for the dialog before this change.

Depends on D63233

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ea05e514940
Fix `JavaScript error: chrome://browser/content/sanitizeDialog.js, line 70: TypeError: right-hand side of 'in' should be an object, got null` error when sanitize dialog is opened outside of preferences;r=Gijs
https://hg.mozilla.org/integration/autoland/rev/844430d0d751
Assign an id to the <window> for the sanitize dialog to fix location persistence;r=Gijs

Comment 6

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/5ea05e514940
https://hg.mozilla.org/mozilla-central/rev/844430d0d751