Closed Bug 633575 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(fennec2.0+)

VERIFIED FIXED
Firefox 4.0
Tracking Status
fennec 2.0+ ---

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Details

(Keywords: polish, Whiteboard: [has-patch])

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
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.
Attachment #511797 - Flags: review?(mark.finkle)
Attached patch patch v2Splinter Review
This gets rid of PreferencesView.init, so that we don't need to load preferences.js during startup at all.
Attachment #511797 - Attachment is obsolete: true
Attachment #511808 - Flags: review?(mark.finkle)
Attachment #511797 - Flags: review?(mark.finkle)
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?
(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().
tracking-fennec: ? → 2.0+
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.
Attachment #511808 - Flags: review?(mark.finkle) → review+
(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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → fennec2.0
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)
Status: RESOLVED → VERIFIED
It seems related to this bug, afaics, bug 627384 is still broken in current trunk build.
You need to log in before you can comment on or make changes to this bug.