436077 - Preferences

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

[Christian Sejersen]

Including support for proxy settings,

Comment 1

[Daniel Brooks [:db48x]]

Since this is still only NEW and it ought to be done before bug 441794 is checked in, I might as well start working on it.

I like the idea from madhava & aza's video for moving away from tabs to a scrollable page listing the prefs.

There are some prefs we can cut if we want, such as the ones for tab behavior. I figure we can easily lose about a quarter of them:

        Main:
nuke      session restore
keep      home page
?         save files location
nuke      addon prefs
nuke    Tabs:
nuke      new pages in
nuke      warnings
        Content:
nuke      block popups
keep      load images auto
          enable javascript
keep      default font/colors...
keep      choose lang...
        Applications:
inline    big list
        Privacy:
keep      keep history for
nuke      accept cookies
keep      private data
        Security:
keep      warn before install
keep      warn on attack/forgery
keep      remember passwords
inline    encryption warnings...
        Advanced:
          General:
add         edit keyboard shortcuts
nuke        cursor keys
nuke        FAYT
nuke        warn on redirect
nuke        autoscroll/smooth scroll
keep        check spelling
keep        default browser
          Network:
inline      proxy...
keep        offline storage/cache space
          Updates:
keep        auto check for updates
keep        auto install updates
keep        update history...
          Encryption:
keep        SSL 3.0/TLS 1.0
keep        certs...

I am, of course, open to suggestions.

Comment 2

[Madhava Enros [:madhava]]

for comparison, here's the full set from mobile safari:

General
	Search Engine - Google/Yahoo

Security
	JavaScript - ON/OFF
	Plug-Ins - ON/OFF
	Block Pop-ups - ON/OFF
	Accept Cookies - Never/From Visited/Always
	Databases - lets you see a list and delete

Clear History (button)
Clear Cookies (button)
Clear Cache (button)
	
Developer
	Debug Console - ON/OFF

and here's what we have in Firefox 3 (large image):

http://people.mozilla.com/~madhava/blog/2008-08-27/full_set_labels2.png

(I have a blog post about this here:
http://madhava.com/egotism/archive/005024.html)

Can we get at something much more like the one in mobile safari -- i.e. radically shorter?

Comment 3

[Daniel Brooks [:db48x]]

Attachment: 436077-1.diff

WIP. This patch doesn't match the mockups Madhava just posted (https://wiki.mozilla.org/Mobile/UI/Designs/TouchScreen/workingUI#Preferences), so I guess it's not really what we want. On the other hand, the UI in this mockup can be generated dynamically which means it should be less work to implement. We'll still be borrowing a lot of the JS from the existing prefs though, since a lot of them need to run js on syncing to and from the backing store to keep things in order.

Comment 4

[Johnathan Nightingale [:johnath]]

(In reply to comment #2)
> Can we get at something much more like the one in mobile safari -- i.e.
> radically shorter?

Starting from Daniel's list, I might submit the following, radically:

Content
 - Load Images
 - Javascript
 - Language [Picker...]

Security & Privacy
 - Clear private data
 - Remember passwords

Advanced
 - Edit keyboard shortcuts
 - Proxy settings

This is deliberately cutty in order to spawn discussion, and because I think a lot of niche needs can be addressed via addons, but here are the motivations line by line (the overarching motivation being "are less than 5% of users ever going to change this option?"):

(In reply to comment #1)
>         Main:
> keep      home page

If new tabs open blank, or with the awesome bar already visible, maybe home page isn't really necessary?

> ?         save files location

File locations are not likely to be a part of a phone-based mental model - I think we pick a smart default and let users override on individual downloads if we think there's value there.

>         Content:
> keep      default font/colors...

I think theming is a better way to go here than a style picker.  The phone's platform will likely have unfamiliar font names, and given the rigidity of phone displays I don't think it's likely that users can ad-hoc their way to success.  I'm sensitive to the accessibility concerns here, above and beyond just aesthetics, but again, I think themes can be built to accomplish this, whereas a pref pane for it feels like it would be painful.  Perhaps I lack imagination, though...

> keep      choose lang...

Safari doesn't have this, but we need it - we are a whole-world browser, I agree.

>         Applications:
> inline    big list

Do phones tend to have the same kind of rich handler-app landscape here?  I dunno, maybe - I defer to the more-mobile-knowing.  At very least, I think it should be at the end. 

>         Privacy:
> keep      keep history for

History is critical to a happy awesomebar experience, and I think the right way to do this is for people who know the platform to make smart calls about the right amount to store, given space constraints on a particular device.  So I don't think we need this option exposed, BUT

> keep      private data

I think it does make sense to keep this option around, for the privacy-conscious.  It's a bit binary, I'll acknowledge that, but I think it is a decent simplicity/functionality trade-off?

>         Security:
> keep      warn before install

We should just leave this on and not expose it, imo.  Hell, I think we should take it out of Firefox's options.  :)

> keep      warn on attack/forgery

Fennec will not have safebrowsing support - it's network intensive when populating the DB, and space-intensive on the device.

> inline    encryption warnings...

The only one of these that defaults to "ON" is low-grade-encryption, which users will not usually encounter.  I think the users who will turn others on overlap almost completely with the group of users who understand about:config.

>         Advanced:
>           General:
> add         edit keyboard shortcuts

I left this in because we've had other bugs talking about how it's essential on certain mobile platforms, otherwise I would have thought it a niche option. 

> keep        check spelling

Again, I think the desirability of spell-checking falls neatly into the 95% plus camp, and doesn't need title billing in our prefs.

> keep        default browser

Fennec, much moreso than Firefox, is almost certainly the default browser when it's running since it will either be the platform load, or (less likely in the initial releases) be the result of some enterprising user going out of their way to install it.  In either case, I think we don't need this toggle exposed, and that if it's anywhere, it's probably a device-level thing.

>           Network:
> inline      proxy...

Yep, devices with WiFi in particular may run the gamut of proxy requirements.

> keep        offline storage/cache space

Dropped this, because I think I remember talk about not doing offline on phones?  Maybe I'm wrong?  Even if not, see above with history - it's worth considering whether we should just pick a smart default on each platform, rather than giving a text box that might let users put in numbers that don't make sense, given the device's storage.  How likely is it that users will make a better call here?

>           Updates:
> keep        auto check for updates
> keep        auto install updates
> keep        update history...

On many platforms, updates will be external (e.g. n810's package manager) but even where they aren't, I think we can default the settings to yes, and nix the update history button.

>           Encryption:
> keep        SSL 3.0/TLS 1.0

I don't think that most users are likely to be able to make better decisions than us here.

> keep        certs...

Whatever we do here, let's punt it to bug 436076 which is about this and only this.

Comment 5

[Madhava Enros [:madhava]]

Thanks guys - those are helpful lists.  Here's what I'd been thinking (with thanks to Beltzner and his pref hatchet):

Content	
- Images 		ON/OFF
- Javascript 	ON/OFF
- Plugins		ON/OFF
- Language		Choose from a picker	(Unless we can foist this off to the OS)

Applications
- Email
- Calendar
maybe others?  but not the huge list

Privacy
- Cookies		ON/OFF
- Clear Private Data	(button)


And then move passwords up to the same level as Add-ons/Downloads/Prefs.

Comment 6

[Madhava Enros [:madhava]]

Regarding proxy settings -- is this something that the OS can do for us?

And I probably should have included "Auto-update ON/OFF" somewhere in there.

Comment 7

[Daniel Brooks [:db48x]]

We can move passwords, applications and shortcut editing up a level. All three are essentially their own (potentially) long list of things, so putting them inside the prefs window is silly, especially with the design you just posted, Madhava.

Comment 8

[Daniel Brooks [:db48x]]

Attachment: 436077-2.diff

90% done, I think. Of course, the other half of the work is in the last 10%; to disable plugins I have to wander around in rdf-land finding them all, and default applications are similarly complicated. Since I can't get either of those working tonight, I figure I'll post what I've got. It's good enough for A1, at any rate.

Clearing private data, on the other hand, does work. It doesn't tell the user that it did anything though.

Comment 9

[Daniel Brooks [:db48x]]

Comment on attachment 337187 [details] [diff] [review]
436077-2.diff

Oh, and a reminder. This patch needs the patch from bug 451025 to work.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Daniel, please see bug 452069 for the new structure of the UI. We should land that bug first, which will bitrot this patch a bit.

Comment 11

[Daniel Brooks [:db48x]]

Attachment: screenshot after integrating with Mark's patch from bug 452069

I won't post the updated patch yet, since it wouldn't apply to a clean build and because Mark's patch is still a WIP.

Comment 12

[Neil Deakin]

Why are the preferences in a richlistbox? It doesn't seem to serve any purpose.

Comment 13

[Daniel Brooks [:db48x]]

The list needs to scroll (if it's long enough, which it isn't yet) and that seemed like the best way to do it.

Comment 14

[Neil Deakin]

You should just be using a scrolling list (scrollbox, or overflow: auto). A richlistbox is used for selecting items.

Comment 15

[Daniel Brooks [:db48x]]

Actually I tried just using overflow: auto/scroll at first, but for some reason it didn't work. I futzed with it a bit, but to no avail. I suppose I could revisit the issue if it's important.

Comment 16

[Stuart Parmenter]

we shouldn't check in code we're unhappy with, so it is probably best to fix the issue neil raises.  also, if we can get this in for the alpha that would be great.

Comment 17

[Daniel Brooks [:db48x]]

Ok, I converted it from a richlistbox to a scrollbox for gavin, and I implemented the pref that enables or disables the plugins. I'll see if I can produce a usable patch, but in the mean time I've got a repo for the changes at http://db48x.net/hg/moz/436077/. Gavin, mfinkle or enn, could you guys review?

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

Do you have diff?

Comment 19

[Daniel Brooks [:db48x]]

Attachment: 436077-3.diff

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 340108 [details] [diff] [review]
436077-3.diff

>diff -r 67c14d267e35 -r 4b9115dbbb29 chrome/content/browser-ui.js
>     var bookmark = document.getElementById("bookmark-container");
>     var urllist = document.getElementById("urllist-container");
>     var container = document.getElementById("browser-container");
>+    var prefs = document.getElementById("pref-pane");

Is this used in the method?

>diff -r 67c14d267e35 -r 4b9115dbbb29 chrome/content/browser.js
>-/* -*- Mode: javascript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
>+// -*- Mode: js2; tab-width: 2; indent-tabs-mode: nil; js2-basic-offset: 2; js2-skip-preprocessor-directives: t; -*-

Will this mess up other people's editors?

>-      // Disable plugins
>-      var phs = Cc["@mozilla.org/plugin/host;1"].
>-                getService(Ci.nsIPluginHost);
>-      var plugins = phs.getPluginTags({ });
>-      for (i = 0; i < plugins.length; ++i)
>-        plugins[i].disabled = true;
>-    }
>+  setPluginState: function(state)
>+  {
>+    var phs = Components.classes["@mozilla.org/plugin/host;1"]
>+                        .getService(Components.interfaces.nsIPluginHost);
>+    var plugins = phs.getPluginTags({ });
>+     for (i = 0; i < plugins.length; ++i)
>+       plugins[i].disabled = state;
>   },

This code was originally a temporary hack. I don't think we want to permanently put this method on Browser. We might want to refactor this code into a PreferenceHelper style object, along with some other prefs code.

If you have no where to put it now, I guess we can keep it here until we refactor.

>diff -r 67c14d267e35 -r 4b9115dbbb29 chrome/content/browser.xul
> <?xml-stylesheet href="chrome://browser/skin/browser.css" type="text/css"?>
> <?xml-stylesheet href="chrome://browser/content/browser.css" type="text/css"?>
>+<?xml-stylesheet href="chrome://browser/skin/richpref.css" type="text/css"?>

Why not put those CSS rules in skin/browser.css? the other embedded panels do for now. If you want them in a separate file, how about just preferences.css and not richprefs.css, which is the XBL name.

>       <deck id="panel-items" flex="1">
>         <iframe id="addons-container" flex="1" src=""/>
>         <iframe id="downloads-container" flex="1" src=""/>
>+
>+        <vbox id="pref-pane">

Nit: id="preferences-container", so it's consistent?

>diff -r 67c14d267e35 -r 4b9115dbbb29 chrome/content/preferences/richpref.xml
>+            try
>+            {
>+              var event = document.createEvent("Events");
>+              event.initEvent(eventName, true, true);
>+              var f = new Function("event", body);
>+              f.call(this, event);
>+            }
>+            catch (e)
>+            {
>+              Components.utils.reportError(e);
>+            }

nit: style is usually open brace on same line - "try {" and "catch {"

Comment 21

[Daniel Brooks [:db48x]]

(In reply to comment #20)
> >diff -r 67c14d267e35 -r 4b9115dbbb29 chrome/content/browser.js
> >-/* -*- Mode: javascript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
> >+// -*- Mode: js2; tab-width: 2; indent-tabs-mode: nil; js2-basic-offset: 2; js2-skip-preprocessor-directives: t; -*-
> 
> Will this mess up other people's editors?

No.

> >+  setPluginState: function(state)
> >+  {
…
> >   },
> 
> This code was originally a temporary hack. I don't think we want to permanently
> put this method on Browser. We might want to refactor this code into a
> PreferenceHelper style object, along with some other prefs code.
> 
> If you have no where to put it now, I guess we can keep it here until we
> refactor.

I left setPluginState in browser.js because it affects the entire browser, and because didn't want to add an entire new file just for one function.

Comment 22

[Daniel Brooks [:db48x]]

Attachment: 436077-4.diff

Comment 23

[Daniel Brooks [:db48x]]

pushed

Comment 24

[Joel Maher ( :jmaher ) (UTC -8)]

verified with beta3