Port the security pane to the in-content preferences implementation

VERIFIED FIXED in Firefox 15

Status

()

Firefox
Preferences
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Jon Rietveld, Assigned: Jon Rietveld)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [testday-20120622])

Attachments

(1 attachment, 9 obsolete attachments)

(Assignee)

Description

5 years ago
move the security pane from preferences into in-content UI preference pane
(Assignee)

Comment 1

5 years ago
Created attachment 607297 [details] [diff] [review]
in-content security pane

Moved Security into in-content UI. In the JavaScript file made changes at like 56-59, 104-107, 190-191, 201-202, and 212-214. All dialog box changes.
Attachment #607297 - Flags: feedback?(jwein)
Attachment #607297 - Flags: feedback?(bmcbride)
Assignee: nobody → jon.rietveld
Blocks: 718011
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Version: unspecified → Trunk
Summary: in-content security pane → Port the security pane to the in-content preferences implementation
Comment on attachment 607297 [details] [diff] [review]
in-content security pane

Review of attachment 607297 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/preferences/in-content/security.xul
@@ +1,5 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +   - You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +    <preferences id="securityPreferences">

This whole file is indented by four characters. Can you please remove this extra indentation?
Attachment #607297 - Flags: feedback?(jwein) → feedback+
(Assignee)

Comment 3

5 years ago
Created attachment 608526 [details] [diff] [review]
security in-content pane

fixed indentation and based off of new landing patch
Attachment #607297 - Attachment is obsolete: true
Attachment #607297 - Flags: feedback?(bmcbride)
Attachment #608526 - Flags: feedback?(jwein)
Attachment #608526 - Flags: feedback?(bmcbride)
(Assignee)

Comment 4

5 years ago
Created attachment 609066 [details] [diff] [review]
security in-content pane

added consistent title
Attachment #608526 - Attachment is obsolete: true
Attachment #608526 - Flags: feedback?(jwein)
Attachment #608526 - Flags: feedback?(bmcbride)
Attachment #609066 - Flags: feedback?(jwein)
Attachment #609066 - Flags: feedback?(bmcbride)
(Assignee)

Comment 5

5 years ago
Created attachment 609227 [details] [diff] [review]
security in-content pane

included search vbox's, and fixed title
Attachment #609066 - Attachment is obsolete: true
Attachment #609066 - Flags: feedback?(jwein)
Attachment #609066 - Flags: feedback?(bmcbride)
Attachment #609227 - Flags: feedback?(jwein)
Attachment #609227 - Flags: feedback?(bmcbride)
Attachment #609227 - Flags: feedback?(jwein) → feedback+
Comment on attachment 609227 [details] [diff] [review]
security in-content pane

Review of attachment 609227 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/preferences/in-content/security.xul
@@ +38,5 @@
> +</vbox>
> +
> +<!-- addons, forgery (phishing) UI -->
> +<vbox data-category="security" hidden="true">
> +  <groupbox id="addonsPhishingGroup">

No need to wrap the <groupbox> in a <vbox> if the <groupbox> is the only child. Just have the data-category and hidden attributes on the <groupbox>, and get rid of the <vbox>.

We really should be doing the same for the heading too - the outer <vbox> is rather redundant :\

@@ +65,5 @@
> +</vbox>
> +
> +<!-- Passwords -->
> +<vbox data-category="security" hidden="true">
> +  <groupbox id="passwordsGroup" orient="vertical">

Ditto.
Attachment #609227 - Flags: feedback?(bmcbride) → feedback+
(Assignee)

Comment 7

5 years ago
Created attachment 613234 [details] [diff] [review]
security in-content patch

fixed blairs feedback
Attachment #609227 - Attachment is obsolete: true
Attachment #613234 - Flags: feedback?(jwein)
Attachment #613234 - Flags: feedback?(bmcbride)

Comment 8

5 years ago
Created attachment 613686 [details] [diff] [review]
Security with updated data-category
Attachment #613234 - Flags: feedback?(jwein) → feedback+
(Assignee)

Comment 9

5 years ago
Created attachment 614506 [details] [diff] [review]
security in-content patch for new landing patch
Attachment #613234 - Attachment is obsolete: true
Attachment #613234 - Flags: feedback?(bmcbride)
Attachment #614506 - Flags: feedback?(jwein)
Attachment #614506 - Flags: feedback?(bmcbride)
(Assignee)

Comment 10

5 years ago
Created attachment 614539 [details] [diff] [review]
security in-content patch for new landing patch
Attachment #614506 - Attachment is obsolete: true
Attachment #614506 - Flags: feedback?(jwein)
Attachment #614506 - Flags: feedback?(bmcbride)
Attachment #614539 - Flags: feedback?(jwein)
Attachment #614539 - Flags: feedback?(bmcbride)
Attachment #614539 - Flags: feedback?(jwein) → feedback+
Attachment #614539 - Flags: review+
Attachment #613686 - Attachment is obsolete: true
Comment on attachment 614539 [details] [diff] [review]
security in-content patch for new landing patch

Review of attachment 614539 [details] [diff] [review]:
-----------------------------------------------------------------

Not sure why, but the master password checkbox is broken - it doesn't update when a master password is added or removed.
Attachment #614539 - Flags: feedback?(bmcbride) → review-
(Assignee)

Comment 12

5 years ago
Created attachment 621674 [details] [diff] [review]
security in-content patch

changed openDialog in changeMasterPassword and _removeMasterPassword to add the feature dlg, which made the dialog open in blocking mode which is needed for the code to function properly
Attachment #614539 - Attachment is obsolete: true
Attachment #621674 - Flags: feedback?(bmcbride)
Comment on attachment 621674 [details] [diff] [review]
security in-content patch

Review of attachment 621674 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/preferences/in-content/security.js
@@ +185,5 @@
> +                          bundle.getString("pw_change2empty_in_fips_mode"));
> +    }
> +    else {
> +      openDialog("chrome://mozapps/content/preferences/removemp.xul",
> +                 "dlg", "modal=yes", null);

"dlg" is not the most awesomest of names. That parameter is used as a sort of ID for the dialog, so it should be unique and descriptive.

So how about "Toolkit:RemoveMasterPassword" for this, and "Toolkit:ChangeMasterPassword" for changemp.xul below.
Attachment #621674 - Flags: feedback?(bmcbride) → review-
(Assignee)

Comment 14

5 years ago
Created attachment 622209 [details] [diff] [review]
security in-content patch

made the changes in comment 13
Attachment #621674 - Attachment is obsolete: true
Attachment #622209 - Flags: review?(bmcbride)
Attachment #622209 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fbf6059c293
Target Milestone: --- → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/0fbf6059c293
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
I verify fixed this bug, tested at Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120621 Firefox/15.0a2.
Status: RESOLVED → VERIFIED
Whiteboard: [testday-20120622]

Updated

4 years ago
Depends on: 949589
You need to log in before you can comment on or make changes to this bug.