633575 - Preference/tools panels are broken if opened during initial page load

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Steps to reproduce:
1. Launch fennec with a URL on the command-line or from another app (easiest with a long-loading page, like planet.mozilla.org).
2. While the page is still loading, use the toolbar or the app menu to open the preferences/add-ons/downloads/console panes.

Expected results: The selected pane is displayed and works.

Actual results: The selected pane is displayed, but it is not initialized. "Language" does not show the right options, add-on/download managers and console do not function.  If you press escape/back to dismiss the panel while the page is still loading, the panel will reappear when the load finishes.

This happens because the "select" event listener to initialize the panel view is not added until after the first page load finishes (UIReadyDelayed).  This was partially hidden by bug 627851 (which caused the preference panel to be initialized anyway), but was still reproducible with the other 3 panels.

This patch ensures that the view always initialized if its panel is selected.  However, we will still wait until UIReadyDelayed to start initializing the view if it is not selected (so regular startup will not be affected).

The patch also simplifies the code by 22 lines, and removes some duplicate work.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

This gets rid of PreferencesView.init, so that we don't need to load preferences.js during startup at all.

Comment 2

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

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

Why do all the delayInit methods call init too. We call init already. Is this to avoid the original issue in the bug? Opening from a commandline url?

Comment 3

[Matt Brubeck (:mbrubeck)]

(In reply to comment #2)
> Why do all the delayInit methods call init too. We call init already. Is this
> to avoid the original issue in the bug? Opening from a commandline url?

Yes, it's because the "select" event might happen before the "UIReadyDelayed" event (if the initial page is still loading), which means delayedInit() will be called before init().

Comment 4

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

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

I kinda want to add a comment above the places where you call this.init() from inside delayedInit() just so we don't mistakenly remove the calls someday.

And put a linebreak below the this.init(); calls to so it stands apart from the delayed init code.

Comment 5

[Matt Brubeck (:mbrubeck)]

(In reply to comment #4)
> I kinda want to add a comment above the places where you call this.init() from
> inside delayedInit() just so we don't mistakenly remove the calls someday.
> 
> And put a linebreak below the this.init(); calls to so it stands apart from the
> delayed init code.

Done.

http://hg.mozilla.org/mobile-browser/rev/411951d404a3

Comment 6

[Anna (Waverley)]

VERIFIED FIXED on:

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110317 Firefox/4.0b13pre Fennec /4.0b6pre 

Devices: Motorola Droid 2 (Android 2.2), Nokia N900 (Maemo GTK)

Comment 7

[Martijn Wargers (dead)]

It seems related to this bug, afaics, bug 627384 is still broken in current trunk build.