231442 - Opening Options from Download Manager defaults to General tab if General is last selected

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P2)

Description

[Alastair Porter]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040118 Firebird/0.7+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040118 Firebird/0.7+

If you go to the download manager and click on the options button on a fresh
profile, the general tab is selected.
This continues untill OK is pressed in the prefs dialog.
If it is dismissed with cancel it will continue

Reproducible: Always

Steps to Reproduce:
1. Delete ~/.phoenix directory
2. Load up firebird (latest-0.8)
3. Open download manager (Tools > Downloads)
4. Click Options

Actual Results:  
The Options dialog opens with the general tab selected

Expected Results:  
The Options dialog should open with the Downloads tab selected

Comment 1

[PikeUK]

After a bit of experimentation this seems to be a more general issue than the
one described in the original report. When the previously selected tab in
Options was the General pane (as it would be on a new profile) then the Download
manager's Options button will not open the Download pane but the General pane.

Build Identifier:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7a) Gecko/20040120 Firebird/0.8.0+

Steps to Reproduce:
1. Open Tools->Options.
2. Select the General pane.
3. Click OK.
4. Open Tools->Downloads.
5. Click Options.

Actual Results:
Options dialog opens with General pane selected.

Expected Results:
Options dialog opens with Downloads pane selected.

The code is failing in the pref-navigator.js Startup() function:

if (top.opener) {
    var browser = top.opener.document.getElementById("content");
    var l = browser.mPanelContainer.childNodes.length;

    if (l > 1) {
      var useButton = document.getElementById("browserUseCurrent");
      useButton.label = useButton.getAttribute("label2");
}

That code makes the assumption that top.opener contains a tabbed browser, but
when the Options window is opened from the Downloads manager that will not be
the case, wrapping the whole lot in a try/catch block fixes the problem.
Presumably there needs to be better checks in that function, though I'm not sure
on the best way to do that.

p.s. I couldn't find an existing bug on this problem, makes me wonder/worry why
no one else has seen such an obvious bug (or did I just miss it in my search),
could someone see if they can reproduce this.

Comment 2

[Mike Connor [:mconnor]]

a try/catch isn't needed, just check if browser exists, e.g.

if (top.opener) {
  var browser = top.opener.document.getElementById("content");
 
  if (browser) {
    var l = browser.mPanelContainer.childNodes.length;

    if (l > 1) {
      var useButton = document.getElementById("browserUseCurrent");
      useButton.label = useButton.getAttribute("label2");
    }
  }
}

actually, we should probably disable the button if browser fails, see bug 236241

patch upcoming

Comment 3

[Mike Connor [:mconnor]]

Attachment: wrap in a try/catch, and disable the current page button if we didn't open from a browser

Comment 4

[Mike Connor [:mconnor]]

Comment on attachment 146798 [details] [diff] [review]
wrap in a try/catch, and disable the current page button if we didn't open from a browser

I have no idea why I'm seeing two review fields here...

Comment 5

[Mike Connor [:mconnor]]

checked in, branch and trunk, 2004-04-26 21:25

Comment 6

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change