Closed
Bug 1074496
Opened 10 years ago
Closed 10 years ago
Disable import from Android in Guest mode
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox35 ?, firefox36 verified, fennec35+)
VERIFIED
FIXED
Firefox 36
People
(Reporter: wesj, Assigned: wesj)
References
Details
Attachments
(1 file)
10.53 KB,
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
This is a potential security breach on the lock screen. I'm pretty close to saying lets just disable all settings:
Assignee | ||
Updated•10 years ago
|
Blocks: fennec-lockscreen
Comment 1•10 years ago
|
||
Setting tracking-35, 'cos that's where lockscreen shipped.
tracking-fennec: --- → 35+
OS: Mac OS X → Android
Hardware: x86 → All
Assignee | ||
Comment 2•10 years ago
|
||
This blocks this pref and the remote debugging pref. Looking through other settings: Customize: Home/Search/Tabs/Updates (why is Updates shown on release?) Nothing looks too dangerous to me here Display Test size/Title bar/Scroll title/Char encoding/Plugins Only plugins sounds dangerous. Not dangerous enough I'm worried though. Privacy: Tracking/Cookies/Remember passwords/Master password/Clear private data Master password seems kinda useless here, but nothing dangerous. Language Nothing bad? Mozilla AboutFirefox/FAQS/Feedback/Product announcements/Telemetry/Crash reporter/Health Report I thought about disabling Telemetry or health report, but I guess they're kinda useful to us? Dev tools Paint flashing
Attachment #8497160 -
Flags: review?(liuche)
Assignee | ||
Updated•10 years ago
|
Attachment #8497160 -
Flags: review?(liuche) → review?(rnewman)
Comment 3•10 years ago
|
||
Comment on attachment 8497160 [details] [diff] [review] Patch Review of attachment 8497160 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me. ::: mobile/android/base/preferences/AndroidImportPreference.java @@ +19,5 @@ > import android.util.Log; > > class AndroidImportPreference extends MultiPrefMultiChoicePreference { > static final private String LOGTAG = "AndroidImport"; > + public static final String PREF = "android.not_a_preference.import_android"; Nit: a better name than PREF? PREF_NAME perhaps? @@ +28,5 @@ > + public boolean setupPref(Context context, Preference pref) { > + return RestrictedProfiles.isAllowed(Restriction.DISALLOW_IMPORT_SETTINGS); > + } > + > + public void onChange(Context context, Preference pref, Object newValue) { } Nit: single space inside braces.
Attachment #8497160 -
Flags: review?(rnewman) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8497160 [details] [diff] [review] Patch Review of attachment 8497160 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/RestrictedProfiles.java @@ +85,1 @@ > if (GeckoAppShell.getGeckoInterface().getProfile().inGuestMode()) { Your tree is out of date. I already fixed this :D ::: mobile/android/base/preferences/AndroidImportPreference.java @@ +25,5 @@ > private Context mContext; > > + public static class Handler implements GeckoPreferences.PrefHandler { > + public boolean setupPref(Context context, Preference pref) { > + return RestrictedProfiles.isAllowed(Restriction.DISALLOW_IMPORT_SETTINGS); Is there a reason for doing this here (creating a new inner class) rather than just continuing with the switch statement removal approach?
Assignee | ||
Comment 5•10 years ago
|
||
No. I just hate that all of our pref handling is all over one massive file full of switch statements. I wanted to pull them all apart at one point in bug 1022103 (also bug 967376 never got finished). If you hate the idea, I'd love to know :)
Comment 6•10 years ago
|
||
I'd like a better approach. (I've thought about just loading prefs screens from different files (or pre-processed files), rather than selectively hiding bits and pieces of a single master screen. That would allow us to do stuff like lift out lonely orphan preferences if a subtree isn't needed, etc. etc.) That said, 80% of a sub-optimal approach and 20% of a good one is perhaps worse than being consistent with the sub-optimal one, but I trust your judgment!
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/c2e81e5b1092
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c2e81e5b1092
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•10 years ago
|
Target Milestone: Firefox 35 → Firefox 36
Is this wanted for Fennec 35?
Comment 10•10 years ago
|
||
Wes: if you want to uplift this to 35, you'll need to apply relevant bits of Bug 1078395 on top of your patch. That's being uplifted first, so some of its changes don't (yet) apply to Aurora. (If you don't, it won't build!)
Flags: needinfo?(wjohnston)
Comment 11•10 years ago
|
||
Verified as fixed in: Build: Firefox for Android 36.a01 (2014-11-09) Device: Nexus 4 (Android 4.4.4)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(wjohnston)
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•