Skip to content

Added onInitalized behaviour and fixed unmet dev dependency#144

Open
nordluf wants to merge 3 commits into
baudehlo:masterfrom
nordluf:master
Open

Added onInitalized behaviour and fixed unmet dev dependency#144
nordluf wants to merge 3 commits into
baudehlo:masterfrom
nordluf:master

Conversation

@nordluf

@nordluf nordluf commented Sep 29, 2016

Copy link
Copy Markdown

Otherwise, there was no possibility to add real "onInitalized"

@nordluf

nordluf commented Dec 5, 2016

Copy link
Copy Markdown
Author

Any objections about this PR?

@nordluf

nordluf commented Apr 13, 2017

Copy link
Copy Markdown
Author

@baudehlo Can you merge this PR? Or tell me - what is wrong with it?

@puzrin

puzrin commented Apr 13, 2017

Copy link
Copy Markdown
Contributor

Why existing callbacks implementation can't be used for this https://github.com/baudehlo/node-phantom-simple/blob/2.2.4/bridge.js#L148?

@nordluf

nordluf commented Apr 13, 2017

Copy link
Copy Markdown
Author

@puzrin Because onInitialized fires before onPageCreated, where all other callbacks set

@baudehlo

baudehlo commented Apr 13, 2017 via email

Copy link
Copy Markdown
Owner

@puzrin

puzrin commented Apr 13, 2017

Copy link
Copy Markdown
Contributor

@baudehlo i don't run it too, because switched to electron.

I could merge something small and clear, but this PR change signatures too much, and adding new hash to param to pass single function looks suspicious. Reviewing this requires deep diving into existing logic and i can't do that now.

@nordluf

nordluf commented Apr 13, 2017

Copy link
Copy Markdown
Author

@puzrin You need to pass that function to createPage method, because only there you can setup onInitialized, right after page object created, before page load process starts. And I tried to do it in a way to keep backward compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants