1667781 - Audit preferences dialogs so default button makes sense

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[:Gijs (he/him)]

The exceptions dialogs (from the privacy & security pane) and the containers preferences dialog box, and maybe some others (?) do not have a "default" button that closes the dialog.

In the case of the exceptions dialog that has an input box, somehow the "Allow" button is the default, which doesn't make a lot of sense...

Comment 1

[:Gijs (he/him)]

Paul, are you OK taking this?

Comment 2

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored ClearSiteData dialog to use dialog element. r=gijs

Comment 3

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored container preferences dialog to use dialog element. r=gijs

Comment 4

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored permission preferences dialog to use dialog element. r=gijs

Comment 5

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored siteDataSettings dialog to use dialog element. r=gijs

Comment 6

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored translation dialog to use dialog element. r=gijs

Comment 7

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored ETP blocklist dialog to use dialog element. r=gijs

Comment 8

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Refactored addEngine dialog to use dialog element. r=gijs

Comment 9

[Tim Nguyen :ntim]

You might want be careful of potential regressions, also to avoid re-indenting to keep the annotate easier to browse (see bug 1585482).

Comment 10

[Emma Zühlcke [:emz]]

(In reply to Tim Nguyen :ntim from comment #9)

You might want be careful of potential regressions, also to avoid re-indenting to keep the annotate easier to browse (see bug 1585482).

Yes, this can lead to regressions. That's also why I've split it up in one patch per dialog so we can track it better. I also ran this on try (all browser mochitests).
Thanks for the suggestion about the indentation. I've fixed that.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/505b2d27fa84
Refactored addEngine dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/3e561d583cd6
Refactored ClearSiteData dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/6f83aae3c18a
Refactored container preferences dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/2864869f2930
Refactored permission preferences dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/d9cb22bc7843
Refactored siteDataSettings dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/9c0ea7b855e0
Refactored translation dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers
https://hg.mozilla.org/integration/autoland/rev/785db1f5301e
Refactored ETP blocklist dialog to use dialog element. r=Gijs,fluent-reviewers,preferences-reviewers

Comment 12

[Emma Zühlcke [:emz]]

Attachment: Bug 1667781 - Added disabled field to SitePermissions gPermissionObject. r=gijs

Depends on D92945

Comment 13

[Phabricator Automation]

Comment on attachment 9180434 [details]
Bug 1667781 - Added disabled field to SitePermissions gPermissionObject. r=gijs

Revision D92946 was moved to bug 1565574. Setting attachment 9180434 [details] to obsolete.

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/505b2d27fa84
https://hg.mozilla.org/mozilla-central/rev/3e561d583cd6
https://hg.mozilla.org/mozilla-central/rev/6f83aae3c18a
https://hg.mozilla.org/mozilla-central/rev/2864869f2930
https://hg.mozilla.org/mozilla-central/rev/d9cb22bc7843
https://hg.mozilla.org/mozilla-central/rev/9c0ea7b855e0
https://hg.mozilla.org/mozilla-central/rev/785db1f5301e

Comment 15

[Phabricator Automation]

Comment on attachment 9180434 [details]
Bug 1667781 - Added disabled field to SitePermissions gPermissionObject. r=gijs

Revision D92946 was moved to bug 1565574. Setting attachment 9180434 [details] to obsolete.