https://www.electronjs.org/fiddle

@sadick254 you have any idea what we do with that command detail there?

hi @darangi, i see we have a fix and we can close this PR now.
I was looking into insight of what we do with this extra command detail in the message.
Any information you can give?

Doesn't this fix #22024 by coincidence?

Not sure.
The issue was atomWindow property not being serializable under electron 8, Atom stop running the right click command event, and not complaining about it at all.

If some of those issue are stopping Atom to complete the tests, yes this pr ( and the merged one ) could have already fixed it.

My solution was to remove it, since everything was working without it.
@darangi solution was to serialize it with structured clone library, but i would love to understand what atom is doing with that property. Because i could not find a use for it.

Still interested in the topic :D anyone available?

Is item.commandDetail.atomWindow used anywhere else in the code?

That is what i was trying to look into. Is not used in atom codebase at all, not with a full text search at least.
I understand the fix we merged for the menu is safer, but it also add a new dependency to the project ( for how much is small ) and to me this seems like the moment to look into it. From git blame it was added in 2014
9128041

But i m not sure how that value is used since the atomWindow is serialized and not more a reference to the original window.

I compiled the version with this commit 5 days ago and i m using it waiting for the fixed nightly to come out.

@sadick254 or @darangi do you have any insight?

@asturur item.commandDetail.atomWindow is not used anywhere in the codebase but passed around and sent as args to the receiver of the context-command action. https://github.com/atom/atom/blob/master/src/main-process/atom-window.js#L378

Since the context menu has already been fixed can we safely close this?

i know is passed down, but if you look at the command, those are just strings and do not seems to use the reference to a copied version of the atomWindow anywhere.
What data is in the atom window we need for the context menu to work?
I do not mind about the PR we can close it, i mind about the conversation if is possible.

• is it used by the plugins ?
• is this a part of the api exposed ouside atom or is just internal?
• was preserving this property the best thing to do?
• is a serialized version of the instance useful at all?

If we can ditch the property and not add a dependency, isn't better?

I have done a little bit of digging into the source code. I can't see anywhere where we use the passed atomWindow reference. Removing atomWindow would be ideal as the serialized version isn't useful at all.

Right now we have a failing CI due to the first context menu fix. Could you include a revert commit of that change on this branch?

Sure i think i can. I got a failing build too and i fixed with the second commit. I ll update it tomorrow early for me

Seems like now this PR is affected by timeouts in windows

Seems like now this PR is affected by timeouts in windows

Yeah, a CI re-run will resolve that CI is ✅

Thanks for the contribution 🙇🏾 @asturur

@asturur Thanks for your perseverance on the topic. This kind of non-trivial contribution is very interesting and valuable!

i stubbornly use atom while evetyone tells me to switch to vscode. I m happy to help and to learn electron.js