-
Notifications
You must be signed in to change notification settings - Fork 46
JSON Object recieving #92
base: master
Are you sure you want to change the base?
Conversation
//Get the element by name. | ||
var data = message.data.split(","); | ||
var iframe = document.getElementsByName(data[0])[0]; | ||
var frameName = msg.frame_id; |
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.
Many messages are sent between many different contexts. Here you should also check that the message you are expecting was sent by looking at the keys in the message. You should also handle improperly formatted JSON.
I think you first need to make a document that gives a (very basic) API between the injected application and the content script (privly.js). Something like: But stated more formally so we can put it on the wiki. After that, you only need to ensure the content script supports the API since the privly-application can decide how to use the commands without you needing to change the apps. |
@smcgregor by making a document that provides an API, you mean a doc, a manual, a readme basically, right? Which tells about various commands that can be sent and the options that can be given for them. |
Yep! |
@smcgregor all right. Done with most of the work, but what's the difference between remove and hide here? |
Remove removes the iframe from the page and hide just doesn't display it.
4 functions is a fine start.
|
@smcgregor and in both cases we replace it with the original URL too? (Actually this was my question) |
Yes.
|
18a2778
to
17bb3bd
Compare
@smcgregor Let me know if things look good now. |
@@ -390,6 +390,9 @@ var privly = { | |||
//Set the source URL | |||
iFrame.setAttribute("src", applicationUrl); | |||
|
|||
//URL which it will be replacing | |||
iFrame.setAttribute("data-privlyHref",object.getAttribute("data-privlyHref")); |
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.
Wasn't this set elsewhere? Why are you setting it now?
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.
@smcgregor In which field is it set? I checked the whole iframe element by going through it, the original link wasn't there. So, I added this and have used it.
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.
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.
@smcgregor Yes. It is set for the element 'a'. And, 'a' is local to that method. I just have access to iframe element here, so had to put the original link as a new attribute into iframe.
Is there a way I could get 'a' corresponding to the iframe directly?
17bb3bd
to
6b151c6
Compare
@smcgregor fixed the pointed out errors and some more nits I observed. |
window.removeEventListener("message", privly.showIframePostedMessage, | ||
false); | ||
|
||
window.removeEventListener("message", privly.hideIframePostedMessage, |
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.
Leftovers.
@smcgregor I'm done with the edits in privly.js. Now, should I add more functions like dispatchResize for other commands in host_page_integration.js and then create unit test n integration tests for those OR, create unit tests and integration tests for privly.js itself, and then add more functions to host_page_integration.js ? |
Although any well written test is a welcome addition, I think it is best to concentrate on the feature set you are developing here. |
Recieving and resizing works. This is in reference to #19 .
However, I'm confused as to what to proceed to at this point. We want two methods
showInjectedFrame
anddestroyInjectedFrame
in host_integration_page.js which have to send another message to privly.js. So, it's more like an extra argument or variable in the JSON Object that privly.js recieves. Am I right?But, what after that?