Preferences gets unchecked temporarily when going to the preferences page

VERIFIED FIXED

Status

()

Firefox for Android
General
P3
normal
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: nhirata, Assigned: bnicholson)

Tracking

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

(Whiteboard: [testday-20111111])

Attachments

(2 attachments, 2 obsolete attachments)

Comment hidden (empty)
1. load preferences
2. watch the checkboxes

Expected: already checked should stay checked on load
Actual: it takes a moment (1.5 seconds) for the checks to show.

http://www.youtube.com/watch?v=ddemMOd_goQ

Motorola Xoom, Droid 2, HTC FLyer
Severity: normal → trivial
Created attachment 573912 [details]
logcat

Based on the logcat I believe that it may be a permissions issue that is slowing things down.
It's probably just a slow read from Gecko over to Java.
Whiteboard: [testday-20111111]

Updated

6 years ago
Severity: trivial → normal
The preferences screen gets initialized asynchronously - when it opens, a message is sent to Gecko to get the preference values, then the response returns back, and the UI is updated. There is a small delay between the request and response with the UI update, which might be a second or two on the slower devices.

My first implementation (bug 694688) cached the preference values in the standard Android shared preferences, which back the the preferences screen, and which the UI gets initial values from. We could re-use that solution if the delay is too annoying.
(In reply to Alex Pakhotin (:alexp) from comment #4)
> The preferences screen gets initialized asynchronously - when it opens, a
> message is sent to Gecko to get the preference values, then the response
> returns back, and the UI is updated. There is a small delay between the
> request and response with the UI update, which might be a second or two on
> the slower devices.

It the read time was pretty noticeable on the Nexus S; I would imagine it being more-so evident on a junkier phone.

Comment 6

6 years ago
brian, can you (1) start the preferences lookup before the activity is created, and (b) can you put up a spinner while this is happening?
Assignee: nobody → bnicholson
Priority: -- → P3
(Assignee)

Comment 7

6 years ago
Created attachment 574736 [details] [diff] [review]
patch

This patch gets the preferences from Gecko immediately after it is ready, and the preferences are kept in memory.  This way, we don't need to issue a Preferences:Get message each time we create the GeckoPreferences activity.
Attachment #574736 - Flags: review?(mark.finkle)
(Assignee)

Comment 8

6 years ago
Created attachment 574742 [details] [diff] [review]
patch v2

removed unused initValues() from GeckoPreferences
Attachment #574736 - Attachment is obsolete: true
Attachment #574736 - Flags: review?(mark.finkle)
Attachment #574742 - Flags: review?(mark.finkle)
Comment on attachment 574742 [details] [diff] [review]
patch v2


>@@ -103,17 +105,17 @@ abstract public class GeckoApp
>     public static File sGREDir = null;
>     public static Menu sMenu;
>     public Handler mMainHandler;
>     private IntentFilter mConnectivityFilter;
>     private BroadcastReceiver mConnectivityReceiver;
>     public static BrowserToolbar mBrowserToolbar;
>     public static DoorHangerPopup mDoorHangerPopup;
>     public Favicons mFavicons;
>-    private static boolean sIsGeckoReady = false;
>+    private static boolean sPrefsLoaded = false;
>     private IntentFilter mBatteryFilter;
>     private BroadcastReceiver mBatteryReceiver;
>     private Geocoder mGeocoder;
>     private Address  mLastGeoAddress;
>     private static LayerController mLayerController;
>     private static GeckoSoftwareLayerClient mSoftwareLayerClient;
>     boolean mUserDefinedProfile = false;
> 
>@@ -393,17 +395,17 @@ abstract public class GeckoApp
>                             }
>                         });
>                     }
>                 }
>                 mi.setOnMenuItemClickListener(item);
>             }
>         }
> 
>-        if (!sIsGeckoReady)
>+        if (!sPrefsLoaded)
>             aMenu.findItem(R.id.preferences).setEnabled(false);

Do we even need this since we are setEnable in the "Preferences:Daata" handler?

I'd like to kill sPrefsLoaded. Can't you init the menu to disabled in gecko_menu.xml

>+            } else if (event.equals("Gecko:Ready")) {
>+
>+                // retrieve the list of preferences from our preferences.xml file

No blank line

>                 connectGeckoLayerClient();
>+
>             } else if (event.equals("PanZoom:Ack")) {

Remove the blank line

>diff --git a/embedding/android/GeckoPreferences.java b/embedding/android/GeckoPreferences.java

>+    private static JSONArray sJsonPrefs = null;

sJsonPrefs -> sJSONPrefs

>     public static void setPreference(String pref, Object value) {
>+
>+        // update the in-memory preferences cache

Remove blank line

r- to see if we can kill sPrefsLoaded
Attachment #574742 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 10

6 years ago
Created attachment 575009 [details] [diff] [review]
patch v3

replaced sPrefsLoaded with GeckoPreferences.isLoaded() and fixed nits.
Attachment #574742 - Attachment is obsolete: true
Attachment #575009 - Flags: review?(mark.finkle)
Comment on attachment 575009 [details] [diff] [review]
patch v3

We could clean this up a little more by moving the initial "make list of preferences, send request to Gecko, listen for return message" into GeckoPreferences too.

File a new bug so we can refactor at some point?
Attachment #575009 - Flags: review?(mark.finkle) → review+
(Assignee)

Updated

6 years ago
Blocks: 703326
(Assignee)

Comment 12

6 years ago
Landed http://hg.mozilla.org/projects/birch/rev/013cc5141612

Filed bug 703326 for cleanup.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Samsung Nexus S (Android 2.3.6)
20111118040220
http://hg.mozilla.org/projects/birch/rev/9999a423d8ab
Status: RESOLVED → VERIFIED
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.