444917 - Port |Bug 388504 – It should be possible for an extension to reload Page Info on a different document| to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, defect)

Description

[Serge Gautherie (:sgautherie)]

I noticed it in bug 405105 comment 2.

While this may not have been the purpose there,
I don't see why we wouldn't want to do it here...

Comment 1

[Philip Chee]

I need to fix this before I can continue on Bug 86400

Comment 2

[Philip Chee]

Attachment: Patch 0.2 WIP.

First cut. This patch could do with some improvement but I'm tired at staring at the code. I'm putting this up for review so that comments can be made.

+function showTab(id)
+  var selectedTab = document.getElementById(id) ||
+                    document.getElementById(id + "Tab") || // Firefox compatibility sillyness 

Just in case some Firefox extension wants to use showTab()

 function onClickMore()
 {
-  var tabbox = document.getElementById("tabbox");
-  var tab = document.getElementById("securityTab");
-  tabbox.selectedTab = tab;
+  showTab("securityTab");

This is called only once. If we don't care about Firefox compatibility at this level we can remove this and call showTab directly.

+++ b/suite/browser/pageinfo/permissions.js

I'm not sure if I have the right number of add/remove observers. I feel all this can be vastly simplified if we can just hide the permissions pane - which is totally useless - for non http(s) pages. FWIW we already hide the feeds tab when there are no feeds.

Comment 3

[[e-mail]]

Comment on attachment 380820 [details] [diff] [review]
Patch 0.2 WIP.

Didn't test this, there's no UI for it yet.

>+  while (feedListbox.firstChild)
Nit: .hasChildNodes()

>+    feedListbox.removeChild(feedListbox.firstChild);
Nit: .lastChild

>+  tabbox.selectedTab = selectedTab;
>+  tabbox.selectedTab.focus();
Nit: don't need tabbox. the second time.

>-  if(/^https?/.test(uri.scheme)) {
>+  if(/^https?/.test(gPermURI.scheme)) {
Might as well fix these to have a space after if while we're here.
[Though I do wonder why it's based on the scheme, not the host...]

Comment 4

[Philip Chee]

Attachment: Patch v1.0 [Checkin: Comment 5]

> (From update of attachment 380820 [details] [diff] [review])
carrying forward r+=Neil

> Didn't test this, there's no UI for it yet.
I have the ExecuteJS extension installed <http://xsidebar.mozdev.org/modifiedmisc.html#executejs>. I open the pageInfo window then navigate to various URLs. Then I run the following code in the context of the pageinfo window:

var xxfoo = {doc: window.opener.gBrowser.contentWindow.document, initialTab: "mediaTab"}
resetPageInfo(xxfoo);

>>+  while (feedListbox.firstChild)
> Nit: .hasChildNodes()
Fixed.

>>+    feedListbox.removeChild(feedListbox.firstChild);
>Nit: .lastChild
Fixed.

>>+  tabbox.selectedTab = selectedTab;
>>+  tabbox.selectedTab.focus();
> Nit: don't need tabbox. the second time.
Fixed.

>>-  if(/^https?/.test(uri.scheme)) {
>>+  if(/^https?/.test(gPermURI.scheme)) {
> Might as well fix these to have a space after if while we're here.
Fixed.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 380862 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 5]


http://hg.mozilla.org/comm-central/rev/86f9ec8efa43