Status

Fennec Graveyard
General
P1
enhancement
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Christian Sejersen, Assigned: db48x)

Tracking

Trunk
fennec1.0a1
Other
Maemo
Dependency tree / graph
Bug Flags:
blocking-xul-fennec1.0 +

Details

(URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
Including support for proxy settings,
(Reporter)

Updated

10 years ago
Assignee: nobody → enndeakin
Priority: -- → P1

Updated

10 years ago
Flags: blocking-fennec1.0+
(Assignee)

Comment 1

10 years ago
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.
Assignee: enndeakin → db48x
Blocks: 441794
(Assignee)

Updated

10 years ago
Depends on: 451025
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
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?
(Assignee)

Comment 3

10 years ago
Created attachment 335902 [details] [diff] [review]
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.
(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.
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.
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.
(Assignee)

Comment 7

10 years ago
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.
(Assignee)

Comment 8

10 years ago
Created attachment 337187 [details] [diff] [review]
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.
Attachment #337187 - Flags: review?(enndeakin)
(Assignee)

Comment 9

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

Oh, and a reminder. This patch needs the patch from bug 451025 to work.
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.
(Assignee)

Updated

10 years ago
Depends on: 452069
(Assignee)

Comment 11

10 years ago
Created attachment 337264 [details]
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

10 years ago
Why are the preferences in a richlistbox? It doesn't seem to serve any purpose.
(Assignee)

Comment 13

10 years ago
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

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

Comment 15

10 years ago
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

10 years ago
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.
(Assignee)

Comment 17

9 years ago
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?
Do you have diff?

Updated

9 years ago
Blocks: 456077
(Assignee)

Updated

9 years ago
Blocks: 454810
(Assignee)

Comment 19

9 years ago
Created attachment 340108 [details] [diff] [review]
436077-3.diff
Attachment #335902 - Attachment is obsolete: true
Attachment #337187 - Attachment is obsolete: true
Attachment #340108 - Flags: review?(mark.finkle)
Attachment #337187 - Flags: review?(enndeakin)
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 {"
Attachment #340108 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 21

9 years ago
(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.
(Assignee)

Comment 22

9 years ago
Created attachment 340213 [details] [diff] [review]
436077-4.diff
Attachment #340108 - Attachment is obsolete: true
Attachment #340213 - Flags: review?(mark.finkle)
Attachment #340213 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 23

9 years ago
pushed
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
No longer blocks: 441794
verified with beta3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.