Extension development - Dynamically set event handler?

Talk about code development, features, specific bugzilla bugs, enhancements, patches, and other highly technical 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 referenced Bugzilla bugs, mercurial, 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. Most "bug reports" do not belong in this board and should initially be posted in Community Support or other relevant support boards.

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.
Post Reply
User avatar
moonbat
Board Warrior
Board Warrior
Posts: 1605
Joined: 2015-12-09, 15:45

Extension development - Dynamically set event handler?

Post by moonbat » 2020-04-15, 14:49

Overlay extension where I'm trying to set the oncommand handler of a command when the browser starts up using document.getElementbyId('command id') instead of hardcoding the javascript function in the XUL file (which overlays browser.xul and loads properly). The XUL file adds a toolbar button and a menuitem, both of which share the command. Both these items are created and visible when I start the browser.

This is done in a function that is invoked during window.onload, defined in the javascript file that is tagged in my overlay XUL file's script tag.
I keep getting 'command id' is not an element in the console.

This is the XUL snippet -

Code: Select all

<overlay id="browser-overlay" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
	<script type="application/x-javascript" src="chrome://myext/content/init.js" />
	<commandset>
		<command id="CmdOptions"/>
	</commandset>
	<!--other stuff--->
	</overlay>
	
This is the event handler and the invoker, which fails at document.getElementById() -

Code: Select all

showOptions: function() {
  Console.log('show options has run.');
},
init : function(){
    document.getElementById("CmdOptions").setAttribute('oncommand','showOptions();');
}
window.addEventListener("load", PureURL.Launch.init(),false);
Where am I going wrong, doesn't the window here refer to the browser window itself, or what am I missing?
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Linux Mint 20 Xfce x64 on HP i5 laptop with 12 GB RAM, always latest versions of PM & Basilisk unless specified.

User avatar
Isengrim
Board Warrior
Board Warrior
Posts: 1166
Joined: 2015-09-08, 22:54
Location: 127.0.0.1
Contact:

Re: Extension development - Dynamically set event handler?

Post by Isengrim » 2020-04-15, 17:12

Not sure why you're getting that error, but try using the DOM Inspector to determine if the command you're referencing exists like you expect it to.

Moreover, rather than try to dynamically change the command element's "oncommand" attribute, why not set the "oncommand" attribute statically to a function that you define in a JS file, and then have that function determine what to do at runtime based on whatever conditions you need? I think that would be much easier than trying to dynamically modify a command element.
Linux Mint 19.3 Cinnamon (64-bit), Debian Bullseye (64-bit), Windows 7 (64-bit), Windows 10 build 1803 (64-bit)
"As long as there is someone who will appreciate the work involved in the creation, the effort is time well spent." ~ Tetsuzou Kamadani, Cave Story

User avatar
New Tobin Paradigm
Off-Topic Sheriff
Off-Topic Sheriff
Posts: 7339
Joined: 2012-10-09, 19:37
Location: Binary Outcast

Re: Extension development - Dynamically set event handler?

Post by New Tobin Paradigm » 2020-04-15, 19:57

Yeah you're trying to be smarter than you are and that is kinda dumb :P

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-15, 20:03

moonbat wrote:
2020-04-15, 14:49
This is the event handler and the invoker, which fails at document.getElementById() -

Code: Select all

showOptions: function() {
  Console.log('show options has run.');
},
init : function(){
    document.getElementById("CmdOptions").setAttribute('oncommand','showOptions();');
}
window.addEventListener("load", PureURL.Launch.init(),false);
Where am I going wrong, doesn't the window here refer to the browser window itself, or what am I missing?
Your problem is in the addEventListener call - you are using "init()" - which means init will be called to return a value to be passed to addEventListener. And at that point, document does not have your overlay loaded. I think you meant to use

Code: Select all

window.addEventListener("load", PureURL.Launch.init,false);
In passing, you should probably be using

Code: Select all

document.getElementById("CmdOptions").addEventListener("command", showOptions);
rather than using setAttribute...

And the 3rd argument to addEventListener defaults to false, so there's no need to supply it.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-15, 20:21

Isengrim wrote:
2020-04-15, 17:12
Not sure why you're getting that error, but try using the DOM Inspector to determine if the command you're referencing exists like you expect it to.

Moreover, rather than try to dynamically change the command element's "oncommand" attribute, why not set the "oncommand" attribute statically to a function that you define in a JS file, and then have that function determine what to do at runtime based on whatever conditions you need? I think that would be much easier than trying to dynamically modify a command element.
not that much easier. in any case, using event listeners is considered to be a better way of doing things, and it's more checkable with linters and such.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-15, 20:23

New Tobin Paradigm wrote:
2020-04-15, 19:57
Yeah you're trying to be smarter than you are and that is kinda dumb :P
and that comment helps him with his actual problem how?

User avatar
New Tobin Paradigm
Off-Topic Sheriff
Off-Topic Sheriff
Posts: 7339
Joined: 2012-10-09, 19:37
Location: Binary Outcast

Re: Extension development - Dynamically set event handler?

Post by New Tobin Paradigm » 2020-04-15, 20:32

And how does your justification of bullshit linters make your suggestion superior to Ascrod's? It doesn't.

You and moonbat are trying to be smarter than you currently are. Take the tried and true method of what Ascrod said and stop trying to treat XUL development like the Modern Web. Because it never will be that. I won't allow it.

Now piss off.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-15, 21:02

You seem to have become rather fond of that phrase today.

In any case I think it'd do more to help people develop for palemoon if instead of insulting them you tried to assist.

User avatar
New Tobin Paradigm
Off-Topic Sheriff
Off-Topic Sheriff
Posts: 7339
Joined: 2012-10-09, 19:37
Location: Binary Outcast

Re: Extension development - Dynamically set event handler?

Post by New Tobin Paradigm » 2020-04-15, 21:22

Moonbat knows if I wanted to insult him, exactly how I would do it. He isn't a complete moron. You though...

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-15, 21:36

Well, I was offered the opportunity to be an incomplete one, but it didn't seem fulfilling

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 27036
Joined: 2011-08-28, 17:27
Location: 58°2'16"N 14°58'31"E
Contact:

Re: Extension development - Dynamically set event handler?

Post by Moonchild » 2020-04-16, 01:21

Off-topic:
Aand with that this exchange has degraded to some stupid back and forth best not posted in public.
STOP. All of you.
I won't warn anymore. If any of you has beef with others (and that includes you, Tobin) take it to private messages, please. On the penalty of cooldowns and/or bans.
"There will be times when the position you advocate, no matter how well framed and supported, will not be accepted by the public simply because you are who you are." -- Merrill Rose
Image

User avatar
moonbat
Board Warrior
Board Warrior
Posts: 1605
Joined: 2015-12-09, 15:45

Re: Extension development - Dynamically set event handler?

Post by moonbat » 2020-04-16, 03:34

thosrtanner wrote:
2020-04-15, 20:03
Your problem is in the addEventListener call - you are using "init()" - which means init will be called to return a value to be passed to addEventListener. And at that point, document does not have your overlay loaded. I think you meant to use

Code: Select all

window.addEventListener("load", PureURL.Launch.init,false);
In passing, you should probably be using

Code: Select all

document.getElementById("CmdOptions").addEventListener("command", showOptions);
Thanks, this fixed it!

I noticed something else - if you assign an ID to the enclosing commandset, the getElementById again fails. I've tried nested calls to it (get element by id for commandset then command, and get element by id (commandset.command) but neither work. Not that I need to assign an ID but would like to know why.

Isengrim wrote:
2020-04-15, 17:12
Moreover, rather than try to dynamically change the command element's "oncommand" attribute, why not set the "oncommand" attribute statically to a function that you define in a JS file, and then have that function determine what to do at runtime based on whatever conditions you need? I think that would be much easier than trying to dynamically modify a command element.
Yeah, that was the original way, but I wanted to keep UI and event handling separate. I've worked on Java Swing based GUIs in the past, and there is separation of UI, event and data (model-view-controller design pattern), so I was expecting to use something like that. Even the MDN docs recommend this.
Off-topic:
I want to make easily maintainable code, from a long term viewpoint. I've been on the receiving end of functional but poorly designed/written and undocumented code myself during my career and it is a nightmare to fix or alter anything. As an example here, I was looking at the Jetpack based Youtube Video Player Pop Out extension on CAA, trying to make an overlay version and it is a giant mess of using Javascript to generate the floating window it displays.
So since I am new to XUL/Javascript as it is, I would rather take time learning to do it right the first time rather than rush to create functionality and clean up the code 'later', which never happens in practice. I'm not on a tight schedule to deliver anyway.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Linux Mint 20 Xfce x64 on HP i5 laptop with 12 GB RAM, always latest versions of PM & Basilisk unless specified.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-16, 06:16

moonbat wrote:
2020-04-16, 03:34

I noticed something else - if you assign an ID to the enclosing commandset, the getElementById again fails. I've tried nested calls to it (get element by id for commandset then command, and get element by id (commandset.command) but neither work. Not that I need to assign an ID but would like to know why.
could you post the code/xul? that looks more like it's caused by a typo or duplicated id rather than anything actually in the system.
Off-topic:
I really would use addEventListener. it's way more flexible than adding an onCommand property, and doesn't have the somewhat odd behaviour with return false. And it doesn't go through an implicit eval which is always a good thing

User avatar
moonbat
Board Warrior
Board Warrior
Posts: 1605
Joined: 2015-12-09, 15:45

Re: Extension development - Dynamically set event handler?

Post by moonbat » 2020-04-16, 09:52

thosrtanner wrote:
2020-04-16, 06:16
could you post the code/xul? that looks more like it's caused by a typo or duplicated id rather than anything actually in the system.
It's the same as my original post, just if you add any random ID to the enclosing commandset. It's the only commandset and there's no duplication.
thosrtanner wrote:
2020-04-16, 06:16
Off-topic:
I really would use addEventListener. it's way more flexible than adding an onCommand property, and doesn't have the somewhat odd behaviour with return false. And it doesn't go through an implicit eval which is always a good thing
Yes, I've switched to using an addEventListener too, what I missed out earlier was that you don't add an 'on' for the event unlike with setting an attribute. So it should've been 'command' not 'oncommand' as I was doing earlier and as you pointed out :)
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Linux Mint 20 Xfce x64 on HP i5 laptop with 12 GB RAM, always latest versions of PM & Basilisk unless specified.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-16, 17:11

Sorry, this might seem a daft question, but I've had a trying day at work.

Are you saying if you do

Code: Select all

<commandset id="fred">
  <command id="jim"/> 
</commandset>
then you can't do document.getElementById("fred")? or "jim"? or both?

User avatar
moonbat
Board Warrior
Board Warrior
Posts: 1605
Joined: 2015-12-09, 15:45

Re: Extension development - Dynamically set event handler?

Post by moonbat » 2020-04-17, 12:48

thosrtanner wrote:
2020-04-16, 17:11
then you can't do document.getElementById("fred")? or "jim"? or both?
Pretty much. Also tried getElementbyId(fred.jim) and getelementbyId(fred).getelementbyid(jim).
Off-topic:
And now I find there is no way to simply get all the elements within a listbox control, either in XUL or standard Javascript. Or if I call slice() on the array of selected elements in a listbox, I'm told that 'it is not a function'. Reminds me why I hate it so much compared to a 'real' programming language that isn't so vague and browser dependent.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Linux Mint 20 Xfce x64 on HP i5 laptop with 12 GB RAM, always latest versions of PM & Basilisk unless specified.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-17, 17:49

I think all the elements in a listbox control might be a NodeList which isn't an array - it's a list. However you can try doing Array.from(thing).slice. But yeah, it'd be nice if things-that-looked-like-arrays occasionally behaved a little more like arrays.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-17, 18:50

I tried this:

Code: Select all

  <toolbox>
    <commandset id="jim">
      <command id="fred"/>
    </commandset>
and did

Code: Select all

console.log(this._document.getElementById("fred"));
console.log(this._document.getElementById("jim"));
and got

Code: Select all

19:45:46.416 <command id="fred">
19:45:46.418 <commandset id="jim">
Is your commandset actually inside a toolbox tag? which is about all I can think of at the moment

User avatar
moonbat
Board Warrior
Board Warrior
Posts: 1605
Joined: 2015-12-09, 15:45

Re: Extension development - Dynamically set event handler?

Post by moonbat » 2020-04-18, 07:17

No, it is in an overlay tag.
"One hosts to look them up, one DNS to find them and in the darkness BIND them."

Linux Mint 20 Xfce x64 on HP i5 laptop with 12 GB RAM, always latest versions of PM & Basilisk unless specified.

thosrtanner
Lunatic
Lunatic
Posts: 290
Joined: 2014-05-10, 18:19
Location: UK

Re: Extension development - Dynamically set event handler?

Post by thosrtanner » 2020-04-19, 11:24

Hmm. i can only make it work inside a toolbox tag inside the overlay tag.

Unfortunately the 'overlays' page in the xul tutorial on MDN appears to have disappeared.

Post Reply