627851 - PreferencesView._delayedInit is not delayed enough

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

We wait for a "select" event on the preference panel to initialize some of the prefs UI.  However, during browser startup, a getBoundingClientRect call causes the panel's XBL constructors to run, and the <menulist> constructors cause "select" events that bubble up to the panel, causing the initialization to run early.

Because the prefs panel is initially selected, it doesn't actually fire any event when you first open the panel.  So we can't just filter the events; we'll need to add a new "panel open" event instead.

Comment 1

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

I assume the getBoundingClientRect call happens after we make the browser tools panels hidden=false (id="panel-container").

Looking at PreferencesView._delayedInit, we don't seem to be doing too much work in there. It could be worse. I am concerned that the XBL constructors are firing while the panel container is still supposed to be hidden. That would be much worse since the <setting>s are slow to init and do hurt Ts. Which is why we made them hidden before.

One simple idea could be to make the <richlistbox id="prefs-list"> hidden by default too. Then in PreferencesView._delayedInit, we could make it hidden=false

Hopefully, the hidden list would not get any XBL initialized.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

I verified that the other panel controllers don't suffer from this bug (they're protected from it because their panels aren't selected during startup).

(In reply to comment #1)
> I assume the getBoundingClientRect call happens after we make the browser tools
> panels hidden=false (id="panel-container").

Looks like that's correct.  _delayedInit is not called until after UIReadyDelayed is fired, so this is out of the path of the first page load (but still earlier than it could be).

> Looking at PreferencesView._delayedInit, we don't seem to be doing too much
> work in there. It could be worse.

Yeah, _delayedInit takes only 7ms on my Galaxy Tab so the impact is pretty small.  (I could imagine us doing more work there in the future, though.)

> I am concerned that the XBL constructors are
> firing while the panel container is still supposed to be hidden. That would be
> much worse since the <setting>s are slow to init and do hurt Ts.

Fortunately, this does not seem to be the case.

Comment 3

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

Comment on attachment 505946 [details] [diff] [review]
patch

nice

Comment 4

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/090018acbd6f

Comment 5

[Andreea Pod]

Is this verified?

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110506 Firefox/6.0a1 Fennec/6.0a1
Device: Droid 2 
OS: Android 2.2

Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110506 Firefox/6.0a1 Fennec/6.0a1
Device: Thunderbolt
OS: Android 2.2