410391 - Clicking on the status-bar security button does not focus on the security tab in the pageinfo window

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, defect)

Description

[Philip Chee]

Attachment: Open the PageInfo window with the correct tab selected.

Steps to reproduce:
1. Navigate to any page.
2. Click on the security button in the status bar.

Actual Results:
The pageinfo window is opened with the general tab selected.

Expected Results:
The pageinfo window is opened with the security tab selected.

Comment 1

[Philip Chee]

> -  tabbox.selectedItem = initialTab;
> -  tabbox.selectedItem.doCommand();
> -  tabbox.focus();
> +  tabbox.selectedTab = initialTab;
> +  tabbox.selectedTab.focus();

Cruft that came over from Firefox:
 .selectedItem doesn't work on <tabbox>.
 There are no oncommand attributes on any of the tabs.

Is .focus() needed here? I can't tell the difference with and without.

Comment 2

[Philip Chee]

Attachment: V1.1 Open the PageInfo window with the correct tab selected.

Neil on #IRC agrees that the .focus() isn't necessary.

Comment 3

[[e-mail]]

Comment on attachment 295012 [details] [diff] [review]
V1.1 Open the PageInfo window with the correct tab selected.

Sorry, but it turns out that I prefer the focus behaviour of attachment 295010 [details] [diff] [review].

I'm also not convinced about the navigator.js change - why not deal with it in page info instead?

Comment 4

[Philip Chee]

> I'm also not convinced about the navigator.js change - why not deal with it in
> page info instead?

API compatibility with Firefox 3.0b2. In porting Firefox PageInfo extensions to SeaMonkey I need fewer if/else blocks. e.g.:
============================================
function viewdep_show()
{
  var depTab = 'DepTab';
  if("@mozilla.org/xre/app-info;1" in Components.classes) {
    var appInfo = Components.classes["@mozilla.org/xre/app-info;1"]
                            .getService(Components.interfaces.nsIXULAppInfo);
    var versionChecker = Components.classes["@mozilla.org/xpcom/version-comparator;1"]
                                   .getService(Components.interfaces.nsIVersionComparator);
    if(versionChecker.compare(appInfo.platformVersion, "1.9b2") >= 0)
      depTab = 'Dep';
  }
  BrowserPageInfo(null, depTab);
}
============================================
If I use the full ID in pageInfo.js then I'll need another check for application as well as the check for Gecko version. By the way this is how I discovered the bug in SeaMonkey's pageInfo. But it's a minor problem. New patch coming up.

Comment 5

[Philip Chee]

Attachment: V1.2 Open the PageInfo window with the correct tab selected (fix nits)

Fix Neil's nits.

Comment 6

[[e-mail]]

Comment on attachment 295015 [details] [diff] [review]
V1.2 Open the PageInfo window with the correct tab selected (fix nits)

Bah, he gives me a good reason, then ignores it...

Comment 7

[[e-mail]]

Comment on attachment 295015 [details] [diff] [review]
V1.2 Open the PageInfo window with the correct tab selected (fix nits)

I guess you can do it this way too if you insist.

Comment 8

[Justin Wood (:Callek)]

Comment on attachment 295015 [details] [diff] [review]
V1.2 Open the PageInfo window with the correct tab selected (fix nits)

Perhaps change this:

>+  initialTab = document.getElementById(initialTab) || document.getElementById("generalTab");

to:

+  initialTab = document.getElementById(initialTab) || document.getElementById(initialTab + "Tab") || document.getElementById("generalTab");

(with appropriate line-wrappings)

This way here, cross UA extensions can work correctly, while we can still keep our code clean and not require the ID trailing with the word "Tab".

(Note, this change won't slow us down, as we pre-store the var "initialTab" as "generalTab" here, so only incorrect ID's will hit all conditions.)

Neil, Ratty, thoughts?

Comment 9

[Philip Chee]

I thought of that, but some extension author might have two elements in their overlay called "foo" and "fooTab" where "foo" might be something else entirely.

Comment 10

[Justin Wood (:Callek)]

could always swap the || order then? :-)

Comment 11

[Philip Chee]

I'm going to file a bug against Firefox to revert to the previous API. Let's see how they react first.

Comment 12

[Philip Chee]

FYI: I've filed Bug 410751 against Firefox to revert to the previous API, so far it's got two r+ and is waiting for a1.9.

Comment 13

[Daniel Brooks [:db48x]]

Comment on attachment 295015 [details] [diff] [review]
V1.2 Open the PageInfo window with the correct tab selected (fix nits)

r=db48x, thanks.

Comment 14

[Reed Loden [:reed]]

Checking in suite/browser/pageinfo/pageInfo.js;
/cvsroot/mozilla/suite/browser/pageinfo/pageInfo.js,v  <--  pageInfo.js
new revision: 1.8; previous revision: 1.7
done