-
Notifications
You must be signed in to change notification settings - Fork 553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Greasemonkey/firefox 58 compability #1249
Conversation
@@ -27,11 +27,11 @@ window.iitcBuildDate = '@@BUILDDATE@@'; | |||
window.onload = function() {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we have the same problem here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like:
unsafeWindow.onload = exportFunction(function() {}, unsafeWindow);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested on Greasemonkey on FF and Tampermonkey on Chrome. I don't think setting window.onload had any use, it has already run at this point. With GM4.0 it isn't assigning the correct "window.onload". It doesn't fail and has no effect since it is not used anyway.
A way to do this is and stay compatible with different extensions and browsers is:
if(typeof exportFunction == "function")
exportFunction(function() {}, window, {defineAs:'onload'});
else if(typeof window.wrappedJSObject == "object")
window.wrappedJSObject.onload = function() {};
else
window.onload = function() {};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good point. That's logically dead code, as is the document.body.onload just below it (introduced by 98f6f19) since document.body as a whole is replaced.
main.js
Outdated
//originally code here parsed the <Script> tags from the page to find the one that defined the PLAYER object | ||
//however, that's already been executed, so we can just access PLAYER - no messing around needed! | ||
|
||
if (typeof(window.PLAYER)!="object" || typeof(window.PLAYER.nickname) != "string") { | ||
var PLAYER = window.PLAYER || window.wrappedJSObject.PLAYER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could just be unsafeWindow in either case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure window.wrappedJSObject exist in all contexts that run IITC ?
Another way to write the test is
if (!((typeof(window.PLAYER)=="object" && typeof(window.PLAYER.nickname) == "string")
||(typeof(window.wrappedJSObject)=="object" && typeof(window.wrappedJSObject.PLAYER)=="object" && typeof(window.wrappedJSObject.PLAYER.nickname) == "string"))){
// page doesn’t have a script tag with player information.
if(document.getElementById('header_email')) {
// however, we are logged in.
// it used to be regularly common to get temporary 'account not enabled' messages from the intel site.
// however, this is no longer common. more common is users getting account suspended/banned - and this
// currently shows the 'not enabled' message. so it's safer to not repeatedly reload in this case
// setTimeout('location.reload();', 3*1000);
throw("Page doesn't have player data, but you are logged in.");
}
// FIXME: handle nia takedown in progress
throw("Couldn't retrieve player data. Are you logged in?");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jormund oh, yes, it needs an extra test for wappedJSObject for TM
with GM there is no "unsafeWindow" anymore workaround: |
@@ -27,11 +27,11 @@ window.iitcBuildDate = '@@BUILDDATE@@'; | |||
window.onload = function() {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine, great job!
can anyone merge this and create a new release, please? |
//originally code here parsed the <Script> tags from the page to find the one that defined the PLAYER object | ||
//however, that's already been executed, so we can just access PLAYER - no messing around needed! | ||
|
||
if (typeof(window.PLAYER)!="object" || typeof(window.PLAYER.nickname) != "string") { | ||
var PLAYER = window.PLAYER || (unsafeWindow && unsafeWindow.PLAYER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var PLAYER = window.PLAYER || (unsafeWindow && unsafeWindow.PLAYER); | |
var PLAYER = window.PLAYER || (typeof unsafeWindow !== 'undefined' && unsafeWindow.PLAYER); |
Otherwise we have TypeError when window.PLAYER
is not defined (which is normal when user is not logged in yet).
closed because stale project |
GM4.0 / FF 58+ -> Content scripts do not have direct access to webpage Javascript content.
see: greasemonkey/greasemonkey#2653 (comment)
this will (atleast) fix the startup error. So far I didn't recognized any other problems.