Need help translating addon SDK code fragment

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-20, 07:54

I started my extension development strictly with the overlay kind, so have no experience with the addon SDK. (I find it particularly convoluted where UI creation goes but that's another story)
While converting an addon SDK extension, it needs to query the windowtype attribute at one place to ensure it equals 'navigator:browser' to ensure it isn't working with some other type of window (say the bookmark manager) and it uses the following function to apply its settings while configureWindow() does nothing but create dynamic entries for the tab context menu.

Code: Select all

function refreshWindows() {
  let windows = require("sdk/windows").browserWindows;
  for (let window of windows) {
    for (let tab of window.tabs) {
      for (let subtab of tab.window.tabs) {
        let win = viewFor(subtab.window);
        configureWindow(win);
        highlightBrowser(viewFor(tab), win);
        refreshButtonIcons(win);
      }
    }
  }
}
As far as I can tell, this first traverses all open browser windows, then all tabs within each window, but I don't get the purpose of the 3rd for loop with subtab.
If configuring the window, then why go down to the level of tabs?

I've got it working with this equivalent -

Code: Select all

	refreshWindows : function(){
		var windows = Services.wm.getEnumerator('navigator:browser');
		var window;
		while(windows.hasMoreElements()){
			window = windows.getNext().QueryInterface(Ci.nsIDOMWindowInternal);
			//window = windows.getNext();
			configureWindow(window);
			for(let tab of window.gBrowser.tabs){
				highlightBrowser(tab,window);
			}
		}
	},
However, highlightBrowser performs the following check in the original -

Code: Select all

function highlightBrowser(aTab, aWindow) {
  if (!isBrowser(aWindow)) {
    return;
  }
...
}
Based on looking up isBrowser() here, I translated the above to :

Code: Select all

	 highlightBrowser : function(aTab, aWindow) {
  		if (aWindow.windowtype != "navigator:browser") {
    		return;
  		}	
  	...
  	}	
but windowtype shows up as undefined. Which leads me to believe I am passing the wrong window type in my version of refreshWindows (in the console, it shows up as a ChromeWindow object).
Where am I going wrong with my version of refreshWindows() :?:
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-20, 22:06

Using DOM Inspector on Pale Moon 29.1.0 (64-bit) shows windowtype as an attribute (not a JavaScript property) on the document.documentElement (node named window, id==main-window) of the browser window whose URL is

Code: Select all

chrome://browser/content/browser.xul
The attribute's value is indeed navigator:browser.

Maybe the SDK extension code is buggy, or at any rate isn't suited to Pale Moon in this regard.

You might try

Code: Select all

aWindow.getAttribute('windowtype')
instead of

Code: Select all

aWindow.windowtype
in your highlightBrowser code.

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-21, 04:15

Thanks, that worked!

It now says getAttribute isn't a function. aWindow here is a ChromeWindow. Which is what prompted my original question, whether the window itself is of the wrong type.

Code: Select all

highlightBrowser : function(aTab, aWindow) {
  		if (aWindow.getAttribute('windowtype') != "navigator:browser") {
  			return;
  		}
...
} 			
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-21, 12:42

Yikes!

Looks like your guess was right; see

Code: Select all

https://udn.realityripple.com/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIDOMWindowInternal
and

Code: Select all

https://udn.realityripple.com/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIDOMWindow
So a quick change to your QueryInterface argument might make getAttribute work...

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-22, 21:41

On further reflection, I doubt whether my last suggestion is enough by itself.

If it's not, then you might try in addition

Code: Select all

aWindow.document.documentElement.getAttribute('windowtype')
in the conditional with which highlightBrowser begins.

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-22, 23:39

This worked! What's the difference here? Does the documentElement actually resolve to a window?
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
Kris_88
Keeps coming back
Keeps coming back
Posts: 932
Joined: 2021-01-26, 11:18

Re: Need help translating addon SDK code fragment

Unread post by Kris_88 » 2021-03-23, 00:09

moonbat wrote:
2021-03-22, 23:39
Does the documentElement actually resolve to a window?
Not to the Window, but to the root element of the document (the <window> element, for example).

Code: Select all

<window id="main-window"
  windowtype="navigator:browser"
  ...
>

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-23, 11:47

Glad it worked!

Curious whether the switch from nsIDOMWindowInternal to nsIDOMWindow was necessary.

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-23, 14:35

No, I left it at nsiDOMWindowInternal.

I am still suspicious whether the window being used in my modified overlay code is the same as the one in the original addon SDK code (the 2 versions of the refreshWindows function in the OP) . I added print statements to both my code and the original, at one point reading the URI of the current tab's browser object in a tab switch listener returns about:blank in my case but the correct URL in the original.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-27, 01:03

Could we see how you obtain the current tab's browser object in the tab switch listener and check its URI?

Also, in connection with your concern about your overlay code using the same windows as the add-on SDK code for refreshWindows : I've been unclear from the outset about the two innermost for loops in the add-on SDK version (but postponed bringing it up in the hopes it wouldn't be necessary). Do you have any idea what a tab.window and a subtab of tab.window.tabs might represent?

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-28, 04:43

opus_27 wrote:
2021-03-27, 01:03
Could we see how you obtain the current tab's browser object in the tab switch listener and check its URI?
I'm using gBrowser.selectedBrowser.currentURI.spec in onLocationChange() for the tab switch listener. I figure it should work because gBrowser is the tabbrowser object of the current window, and selectedBrowser ought to be the browser of the tab currently in focus.
opus_27 wrote:
2021-03-27, 01:03
the two innermost for loops in the add-on SDK version
That is what has me confused as well, as mentioned in my first post. As far as I understand, you need one outer loop to traverse through all valid browser windows and add the dynamic items to the tab context menu (in configureWindow() ) , then one inner loop through the tabbed browser of each window to apply CSS border color highlights to each tab (in highlightBrowser() ).
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-28, 11:54

Sorry if the following questions seem redundant, but I may be misunderstanding you:
  1. Could you please verify whether your onLocationChange() is a member of an implementation of nsIWebProgressListener (as indicated in lines 334-380)?
  2. Could you also verify that by "tab switch listener" you mean a function/method called, for example, when the user changes tabs by clicking on one other than the currently selected one or using ctrl-Tab?

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-28, 13:12

Yes to both :)
Actually let me link you to the project source itself. sjoverlay.js is the overlay into browser.xul, and sjcore and sjcommon.jsm have common functions.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
opus_27
Apollo supporter
Apollo supporter
Posts: 36
Joined: 2020-06-16, 13:29

Re: Need help translating addon SDK code fragment

Unread post by opus_27 » 2021-03-28, 16:24

Thanks for the answers and the link.

I suggest you try this: in init.tabSwitchListener.onLocationChange (sjoverlay.js line 278-284), drop the first statement (let...;) and change the second to

Code: Select all

Straitjacket.Init.highlightBrowser(null,window.document.documentElement);
This should work, since each gBrowser listener will hear only from tabs in its own "main-window"; if it does work, you can try omitting window.

I hope this makes sense.

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-29, 04:00

That didn't work - aWindow.getAttribute() now fails in highlight browser. One thing I don't get - since this is a browser.xul overlay, wouldn't it apply separately to each separate browser window, including each window's separate gBrowser?
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35405
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Need help translating addon SDK code fragment

Unread post by Moonchild » 2021-03-29, 04:15

Keep in mind that many attributes in XUL are mapped to properties. You may not even need to use getAttribute() (making it a lot simpler and cheaper)

See e.g. https://developer.mozilla.org/en-US/doc ... m_elements

Not having time to look into what you're actually trying to do, so i may be completely missing the point, the obvious suggestion I think would be to try using the windowManager interface? That should give you all the methods needed to enumerate and iterate through windows and tabs.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-29, 04:37

Moonchild wrote:
2021-03-29, 04:15
the obvious suggestion I think would be to try using the windowManager interface?
Do you mean nsiWindowMediator? It can be invoked via Services.wm, can't find a reference to any other windowmanager interface, and I used that to enumerate all browser windows before looping through them to apply the extensions additions to the tab context menu.

My concern is the discrepancy in behavior between the original addon SDK extension and mine that's trying to turn it into a classic overlay extension, so I am not sure whether the references to windows and tabs in the original are being used correctly in my version. I see a ChromeWindow being passed in some cases and I don't know if that is the right one, even though the menus and UI is all created correctly.

The biggest problem I've seen is using the session restore API. As with the original, I have added restore listeners where it is supposed to check whether the tab being restored is a sandboxed tab and then reload the sandbox if it had had one originally. This works in the original extension but not in mine despite the restoreTab method in sjcore.jsm being fairly identical to the original.

One more thing - the original extension checks for e10s in a couple of places by checking whether the message manager is present and does things differently. Should I retain these checks or remove them and assume non e10s always? I'm asking because I see the message manager does exist so Pale Moon probably has some e10s support left over.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35405
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Need help translating addon SDK code fragment

Unread post by Moonchild » 2021-03-29, 05:06

Yes I may be mixing up names here, sorry if so.

If you're really stuck though, you do realize that the SDK is an abstraction layer, right? you can look at the SDK code itself and see how it is abstracting these functions from available interfaces to see what you need.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
moonbat
Knows the dark side
Knows the dark side
Posts: 4942
Joined: 2015-12-09, 15:45
Contact:

Re: Need help translating addon SDK code fragment

Unread post by moonbat » 2021-03-29, 05:15

Thanks, that's an idea.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Image
Linux Mint 21 Xfce x64 on HP i5-5200 laptop, 12 GB RAM.
AutoPageColor|PermissionsPlus|PMPlayer|Pure URL|RecordRewind|TextFX

Locked