699418 - Implement about:config in html

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Lawrence Mandel [:lmandel] (use needinfo)]

I tried opening about:config on native UI nightly. I can select the preferences that are in view but can't scroll the page to select other preferences.

Comment 1

[Doug Turner (:dougt)]

we probably should just rewrite about:config in html and be done.

Comment 2

[:Margaret Leibovic]

(In reply to Doug Turner (:dougt) from comment #1)
> we probably should just rewrite about:config in html and be done.

I agree with this. I was looking into bug 699460, and there seems to be a bunch of problems with the xul widgets. That bug is about boolean setting widget not working, but I found other fundamental problems (for example, the input box when you try to create a new preference is less than 1 character wide so you can't see what you're typing).

Comment 4

[:Margaret Leibovic]

Attachment: patch v1

This makes a really basic UI, but it's functional. I started out just trying to port logic from the XUL version, but I got frustrated trying to make that work because it was overly complex for what I was trying to do, so I ended up writing a simpler version of it.

Things that could be better but probably aren't necessary right now:
-You have to press the "Filter" button to filter the results. I tried making this into a form so that you would get a "Go" in the keyboard to submit it, but I had trouble getting that to work right.
-When you add new prefs they just get added to the top of the list instead of in their correctly sorted spot. I could try adding them in the right spot, but then we'd also probably want to scroll to that spot, and I just didn't feel like dealing with that :)
-At some point we could add back inline editing, but that should definitely be a separate (low priority) bug.

I was gonna give this one final test, but I updated my build and now it's crashing... so I'll test it more once I get that figured out.

Comment 5

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

Comment on attachment 575384 [details] [diff] [review]
patch v1

Very nice!

>diff --git a/mobile/android/chrome/content/config.xhtml b/mobile/android/chrome/content/config.xhtml

>+      filter: function AC_filter(aValue) {

>+          // Show all the items
>+          for (let i = 0; i < items.length; i++) {
>+            items[i].removeAttribute("hidden");
>+          }

>+            // Hide all the items
>+            for (let i = 0; i < items.length; i++) {
>+              items[i].setAttribute("hidden", "true");
>+            }

>+          for (let i = 0; i < items.length; i++) {
>+            let item = items[i];
>+            if (reg.test(item.getAttribute("name") + ";" + item.getAttribute("value")))
>+              item.removeAttribute("hidden");
>+            else
>+              item.setAttribute("hidden", "true");
>+          }

Testing on my nexus one, I saw a noticeable delay when trying to filter. I also saw the Java compositor not update after a filter was completed either, but that's not your bug.

Walking the DOM and using the "hidden" attribute might be slowing you down. It's not critical for the initial landing, but we should try to address this. A couple of ideas:

* Hide the "prefs-container" DIV (and show with "Searching..." DIV) until the filter is complete. This lets me know something is happening and doesn't appear hung.
* Since you put the name of the preference in an attribute, I wonder if we could add a CSS rule, dynamically, to the stylesheet which would quickly show/hide elements?

div.pref-item:not([name*='the-filter-value']) {
  visibility: hidden;
}

You could make this style and add/remove it from the stylesheet as needed. If the filter string is empty, just remove it, otherwise add it. I don't know how much faster this would be, but I think it would avoid reflows that happen each time you change "hidden" in JS.

Also, I'm acting like I know this will even work, I don't :)

>+      modifyPref: function AC_modifyPref(aPref) {

>+            let supportsString = Cc["@mozilla.org/supports-string;1"].
>+                                 createInstance(Ci.nsISupportsString);

no wrap needed

>+      _setupPrefs: function AC_setupPrefs() {

>+        let prefs = list.sort().map(this._getPref, this);
>+        for (let i = 0; i < prefs.length; i++) {
>+          let item = this._createItem(prefs[i]);
>+          this._container.appendChild(item);
>+        }

This can take several seconds when loading the page. Try using a document fragment approach, like this:
http://mxr.mozilla.org/mozilla-central/source/mobile/xul/chrome/content/common-ui.js#768

It should make things load/render faster, since only one reflow will happen.

>+      _getPref: function AC_getPref(aPrefName) {
>+        let pref = {
>+                     name: aPrefName,
>+                     value:  "",
>+                     default: !Services.prefs.prefHasUserValue(aPrefName),
>+                     lock: Services.prefs.prefIsLocked(aPrefName),
>+                     type: Services.prefs.getPrefType(aPrefName)
>+                   };

Not your code, but could you reformat this to:

          let pref = {
            name: aPrefName,
            value:  "",
            default: !Services.prefs.prefHasUserValue(aPrefName),
            lock: Services.prefs.prefIsLocked(aPrefName),
            type: Services.prefs.getPrefType(aPrefName)
          };

>diff --git a/mobile/android/locales/en-US/chrome/config.properties b/mobile/android/locales/en-US/chrome/config.properties
>+addPref.title=Add Preference

change to "Add" (since I am suggesting shorter labels below and you use those labels for the prompt title, this makes things consistent)

>+addPref.selectType=Select preference type

change to "Select type:"

>+addPref.enterName=Enter preference name

change to "Enter name:"

>+modifyPref.label=Modify Preference

change to "Modify"

>+resetPref.label=Reset Preference

change to "Reset"

Later, after this lands, we could also look at hiding.showing the "Modify" / "Reset" buttons on tap - might make the list easier to read.

r- so I can see a new patch, but this is very usable. Can't wait to get it in the product.

Comment 6

[:Margaret Leibovic]

Attachment: patch v2

I decided to try just replacing the DOM on filter, the same way the old about:config does it, and it feels much faster (although that's probably also due to the document fragment). This also feels cleaner than all the hiding/showing logic.

Comment 7

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

Comment on attachment 575554 [details] [diff] [review]
patch v2

Nice work. I'm sure we'll get feedback and feature requests, but this is a big improvement.

Comment 8

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> Nice work. I'm sure we'll get feedback and feature requests, but this is a
> big improvement.

Yay! Yeah, we can file follow-ups for more work.

https://hg.mozilla.org/projects/birch/rev/80569c9d351d

Comment 9

[Catalin Suciu [:csuciu]]

Verified fixed on the latest fennec native build:
Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111122 Firefox/11.0a1 Fennec/11.0a1
Samsung GalaxyS, Android 2.2