Phantomjs: integrate ghostdriver changes into phantomjs

Created on 21 Sep 2016  路  29Comments  路  Source: ariya/phantomjs

@ariya i'd like to discuss how the phantomjs project can integrate changes from ghostdriver in a stable fashion.

why i care

ghostdriver has not been updated in phantomjs for over two years. this is a problem. for instance cookies are broken in ghostdriver in the 2.1.1 phantomjs release. the cookie tests in ghostdriver fail so this should have been caught.

we can do a few things to integrate stable changes from ghostdriver:

  1. run the java unit tests in the ghostdriver project
  2. run the selenium/webdriver test suite on phantomjs
  3. a beta release of phantomjs so the selenium users can test changes without having to build the project from source

while (2) is valuable it is harder to do than (1). also ghostdriver is not 100% compatible with webdriver specification so some of the selenium/webdriver tests will fail.

picture jesg
👍4

Most helpful comment

@jesg Nice! I guess we are ready to release 2.5

picture vitallium on 1 Apr 2017
❤5

All 29 comments

the w3c project web platform tests might be a good way to run selenium tests. more info at mozilla

jesg picture jesg on 21 Sep 2016

@jesg Do you think it's possible to use Ghost Driver as a stand-alone tool, i.e. alongside PhantomJS? That way, it can be updated more frequently without the need to wait for a new PhantomJS release.

ariya picture ariya on 22 Sep 2016

Do you think it's possible to use Ghost Driver as a stand-alone tool, i.e. alongside PhantomJS?

@ariya yes. the java client can already does that when the user sets the phantomjs.ghostdriver.path capability. this is not available in the python, javascript, or the ruby client. the current design requires changes to each of the phantomjs selenium clients upstream.

i wrote a little wrapper script in bash which was good enough for me to test ghostdriver changes in python. the same thing can be done in javascript.

i would prefer an environment variable or a command line argument to specify the location of ghostdriver's startup script. this would make it easier to change and not worry about messing up the command line arguments.

npm can be used to package and distribute ghostdriver.

jesg picture jesg on 22 Sep 2016

So what if for the next version (2.5), you integrate whatever the latest stable of Ghost Driver is into PhantomJS, and at the same time, provide a command-line argument to point to an external Ghost Driver. This can serve as a transition step, at some point in the future we can remove the built-in prepackaged Ghost Driver completely from PhantomJS.

Thoughts?

ariya picture ariya on 23 Sep 2016

Thoughts?

sounds like a plan. i'll start working on it.

should the command line option be --webdriver-ghostdriver-path=<val>?

jesg picture jesg on 23 Sep 2016
👍2

Based off the other command line options, I'd expect the option to be --webdriver-path=<val>, as there's only one possible webdriver to use with PhantomJS (GhostDriver) and I'm not sure there's plans to try and incorporate other ones?

MasterOdin picture MasterOdin on 23 Sep 2016

Looking forward to this! :)

henrik242 picture henrik242 on 26 Sep 2016

@jesg @ariya How is the progress? :)

henrik242 picture henrik242 on 7 Oct 2016

@henrik242 i made two patches, one for 2.1.1 and one for 2.5. i was able to compile and verify my patch for 2.1.1.

i'm waiting for ariya/phantomjs#14459 (or some instructions on how to build everything from source) to be resolved so i can build the master branch.

jesg picture jesg on 7 Oct 2016
👍4

@Vitallium @ariya i saw some regressions when i ran the phantomjs test suite. where is the best place to work through these failures?

i built qt5.6.1, Vitallium/phantomjs master branch + fix for encoding and annulen/webkit qtwebkit-tp4 tag on ubuntu 16.04.

jesg picture jesg on 12 Dec 2016

:+1: some communities started to recode a new webdriver ! :( (ruby/php)

Nek- picture Nek- on 14 Dec 2016

@jesg that's okay actually

vitallium picture vitallium on 14 Dec 2016

@Vitallium the page url after a redirect is not correct. url-encoding.js#L16 should reproduce the issue.

jesg picture jesg on 15 Dec 2016

the initial (unofficial) npm package is complete https://www.npmjs.com/package/ghostdriver.

run the following to install with the phantomjs-prebuilt package.

npm install --global ghostdriver

run the following to install without the phantomjs-prebuilt package.

npm install --global --no-optional ghostdriver
jesg picture jesg on 18 Dec 2016

@jesg awesome! What about integrating the latest Ghostdriver to the main source tree?

vitallium picture vitallium on 18 Dec 2016

@Vitallium

What about integrating the latest Ghostdriver to the main source tree?

i need to work through some regressions ghostdriver has on phantomjs 2.5 before submitting the pull request. i can submit a pull request with what i have so far if you want to just get it out there.

regressions i've found so far:

  • all forms of file upload do not work (i need to triage this and reproduce in pure phantomjs) has the webkit api changed?
  • slight change in page loading behavior (see loading.js) ... i fixed this by switching to webpage#loadingProgress.
  • ghostdriver's "best effort" approach is not good enough to detect a page load after a click event
  • javascript closing a window causes ghostdriver to freeze (i need to reproduce in pure phantomjs)
  • url is not correct after a redirect (see url-encoding.js)
jesg picture jesg on 19 Dec 2016

here are the selenium results. this does not include any of the file upload tests or javascript deleting window tests both of which freeze phantomjs. fwiw the file upload failures may be indirectly related to org.openqa.selenium.MiscTest#shouldReturnTagName failure.

according to the selenium project the failures in CorrectEventFiringTest, ElementFindingTest, FrameSwitchingTest, JavascriptEnabledDriverTest, PageLoadingTest, and SlowLoadingPageTest are important. for the sake of time i plan on ignoring anything that was failing in 2.1.1.

org.openqa.selenium.ClickTest#testShouldBeAbleToClickOnASpanThatWrapsToTheNextLine
org.openqa.selenium.ClickTest#testShouldOnlyFollowHrefOnce
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoTinyMCE
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoEmptyContentEditableElement
org.openqa.selenium.ContentEditableTest#testTypingIntoAnIFrameWithContentEditableOrDesignModeSet
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoContentEditableElementWithExistingValue
org.openqa.selenium.ContentEditableTest#testNonPrintableCharactersShouldWorkWithContentEditableOrDesignModeSet
org.openqa.selenium.ContentEditableTest#testShouldAppendToTinyMCE
org.openqa.selenium.CorrectEventFiringTest#testNativelyClickOverlappingElements
org.openqa.selenium.CorrectEventFiringTest#testClickOverlappingElements
org.openqa.selenium.CorrectEventFiringTest#testClickAnElementThatDisappear
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyTagNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyClassNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingMultipleElementsByEmptyNameShouldReturnEmptyList
org.openqa.selenium.ElementFindingTest#testFindingMultipleElementsByEmptyIdShouldReturnEmptyList
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyIdShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyCssSelectorShouldThrow
org.openqa.selenium.FrameSwitchingTest#testGetCurrentUrlReturnsTopLevelBrowsingContextUrl
org.openqa.selenium.FrameSwitchingTest#testGetCurrentUrlReturnsTopLevelBrowsingContextUrlForIframes
org.openqa.selenium.I18nTest#testShouldBeAbleToActivateIMEEngine
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldBeAbleToClickALinkThatClosesAWindow
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldFireOnChangeEventWhenSettingAnElementsValue
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldBeAbleToClickOnSubmitButtons
org.openqa.selenium.JavascriptEnabledDriverTest#testIssue80ClickShouldGenerateClickEvent
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldBeAbleToFindElementAfterJavascriptCausesANewPageToLoad
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldBeAbleToSwitchToFocusedElement
org.openqa.selenium.JavascriptEnabledDriverTest#testChangeEventIsFiredAppropriatelyWhenFocusIsLost
org.openqa.selenium.JavascriptEnabledDriverTest#testIfNoElementHasFocusTheActiveElementIsTheBody
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldWaitForLoadsToCompleteAfterJavascriptCausesANewPageToLoad
org.openqa.selenium.MiscTest#testShouldReturnTheSourceOfAPage
org.openqa.selenium.MiscTest#testClickingShouldNotTrampleWOrHInGlobalScope
org.openqa.selenium.MiscTest#testShouldBeAbleToGetTheSourceOfAnXmlDocument
org.openqa.selenium.MiscTest#shouldReturnTagName
org.openqa.selenium.MiscTest#testShouldReportTheCurrentUrlCorrectly
org.openqa.selenium.MiscTest#testShouldReturnTitleOfPageIfSet
org.openqa.selenium.ObjectStateAssumptionsTest#testInitializedWebDriverDoesNotThrowNPE
org.openqa.selenium.PageLoadingTest#testShouldBeAbleToLoadAPageWithFramesetsAndWaitUntilAllFramesAreLoaded
org.openqa.selenium.PageLoadingTest#testShouldNotHangIfDocumentOpenCallIsNeverFollowedByDocumentCloseCall
org.openqa.selenium.PageLoadingTest#testShouldDoNothingIfThereIsNothingToGoBackTo
org.openqa.selenium.PageLoadingTest#testShouldTimeoutIfAPageTakesTooLongToRefresh
org.openqa.selenium.PageLoadingTest#testShouldTimeoutIfAPageTakesTooLongToLoadAfterClick
org.openqa.selenium.PositionAndSizeTest#testShouldGetCoordinatesInViewPortOfAnElementInAFrame
org.openqa.selenium.PositionAndSizeTest#testShouldGetCoordinatesInViewPortOfAnElementInANestedFrame
org.openqa.selenium.SlowLoadingPageTest#testRefreshShouldBlockUntilPageLoads
org.openqa.selenium.SlowLoadingPageTest#testShouldBlockUntilPageLoads
org.openqa.selenium.VisibilityTest#testShouldNotBeAbleToTypeToAnElementThatIsNotDisplayed
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMoveRelativeToBody
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testShouldClickElementInIFrame
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMovingMouseBackAndForthPastViewPort
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMousePositionIsNotPreservedInActionsChain
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMovingIntoAnImageEnclosedInALink
org.openqa.selenium.interactions.CombinedInputActionsTest#testShiftClickingOnMultiSelectionList
org.openqa.selenium.interactions.CombinedInputActionsTest#testCombiningShiftAndClickResultsInANewWindow
org.openqa.selenium.interactions.DragAndDropTest#testDragAndDropElementWithOffsetInIframeAtBottom
org.openqa.selenium.interactions.DragAndDropTest#testDragAndDropToElementInIframe
jesg picture jesg on 21 Dec 2016

commit 7bd1fc86c6af16712c9ec95bfc2567e8d1ea4788 should fix the majority of the regressions i saw.

here are the results skipping file upload tests and window switching tests.

org.openqa.selenium.ClickTest#testShouldBeAbleToClickOnASpanThatWrapsToTheNextLine
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoTinyMCE
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoEmptyContentEditableElement
org.openqa.selenium.ContentEditableTest#testTypingIntoAnIFrameWithContentEditableOrDesignModeSet
org.openqa.selenium.ContentEditableTest#testShouldBeAbleToTypeIntoContentEditableElementWithExistingValue
org.openqa.selenium.ContentEditableTest#testNonPrintableCharactersShouldWorkWithContentEditableOrDesignModeSet
org.openqa.selenium.ContentEditableTest#testShouldAppendToTinyMCE
org.openqa.selenium.CorrectEventFiringTest#testNativelyClickOverlappingElements
org.openqa.selenium.CorrectEventFiringTest#testClickOverlappingElements
org.openqa.selenium.CorrectEventFiringTest#testClickAnElementThatDisappear
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyTagNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyClassNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingMultipleElementsByEmptyNameShouldReturnEmptyList
org.openqa.selenium.ElementFindingTest#testFindingMultipleElementsByEmptyIdShouldReturnEmptyList
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyNameShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyIdShouldThrow
org.openqa.selenium.ElementFindingTest#testFindingASingleElementByEmptyCssSelectorShouldThrow
org.openqa.selenium.FrameSwitchingTest#testGetCurrentUrlReturnsTopLevelBrowsingContextUrl
org.openqa.selenium.FrameSwitchingTest#testGetCurrentUrlReturnsTopLevelBrowsingContextUrlForIframes
org.openqa.selenium.I18nTest#testShouldBeAbleToActivateIMEEngine
org.openqa.selenium.JavascriptEnabledDriverTest#testShouldBeAbleToFindElementAfterJavascriptCausesANewPageToLoad
org.openqa.selenium.PageLoadingTest#testShouldTimeoutIfAPageTakesTooLongToRefresh
org.openqa.selenium.PositionAndSizeTest#testShouldGetCoordinatesInViewPortOfAnElementInAFrame
org.openqa.selenium.PositionAndSizeTest#testShouldGetCoordinatesInViewPortOfAnElementInANestedFrame
org.openqa.selenium.VisibilityTest#testShouldNotBeAbleToTypeToAnElementThatIsNotDisplayed
org.openqa.selenium.WindowSwitchingTest#initializationError
org.openqa.selenium.WindowTest#initializationError
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMoveRelativeToBody
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testShouldClickElementInIFrame
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMovingMouseBackAndForthPastViewPort
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMousePositionIsNotPreservedInActionsChain
org.openqa.selenium.interactions.BasicMouseInterfaceTest#testMovingIntoAnImageEnclosedInALink
org.openqa.selenium.interactions.CombinedInputActionsTest#testShiftClickingOnMultiSelectionList
org.openqa.selenium.interactions.CombinedInputActionsTest#testCombiningShiftAndClickResultsInANewWindow
org.openqa.selenium.interactions.DragAndDropTest#testDragAndDropElementWithOffsetInIframeAtBottom
org.openqa.selenium.interactions.DragAndDropTest#testDragAndDropToElementInIframe

@Vitallium phantomjs 2.5 is having issues resolving relative urls when calling window.open. in my tests <a id="new_window" onmouseup="window.open('closeable_window.html', 'close_me')" href="#">Click me to open a new window</a> will attempt to open a local file. it should be opening a relative url.

i've been unable to get file upload working. i might have compiled webkit wrong. i see the following in webkit source. is there any way for me to force QT_NO_FILEDIALOG to be true?

bool QWebPage::extension(Extension extension, const ExtensionOption *option, ExtensionReturn *output)               
{                                                                                                                   
#ifndef QT_NO_FILEDIALOG                                                                                            
    if (extension == ChooseMultipleFilesExtension) {                                                                
        // FIXME: do not ignore suggestedFiles                                                                      
        QStringList suggestedFiles = static_cast<const ChooseMultipleFilesExtensionOption*>(option)->suggestedFileNa     mes;                                                                                                                
        QStringList names = QFileDialog::getOpenFileNames(view(), QString::null);                                   
        static_cast<ChooseMultipleFilesExtensionReturn*>(output)->fileNames = names;                                
        return true;                                                                                                
    }                                                                                                               
#endif                                                                                                              

    return false;                                                                                                   
}
jesg picture jesg on 27 Dec 2016

@jesg I've fixed the problem with relative URLs. What's ETA on Ghostdriver PR?

vitallium picture vitallium on 22 Feb 2017

@Vitallium where is your fix?

~have you been able to look at the change in WebPage::open and WebPage::loading? loading returned true immediately after the call to open in 2.1.1. ghostdriver used this to detect a page load when user clicks a link or submits a form. my current workaround uses onLoadStarted and loadingProgress. how would you recommend detecting a page load in phantomjs 2.5?~

What's ETA on Ghostdriver PR?

~2 weeks after i get your fix.

jesg picture jesg on 22 Feb 2017

@Vitallium @ariya my changes to ghostdriver are ready for review. my changes should be retested after @Vitallium's relative url fix is merged in.

jesg picture jesg on 26 Feb 2017

Excellent work, thank you so much @jesg!

From my perspective, LGTM. What do you think @Vitallium?

ariya picture ariya on 26 Feb 2017

@ariya LGTM!
@jesg I'll put the patch with the fix for WebKit today.

vitallium picture vitallium on 27 Feb 2017

@Vitallium things are moving along downstream. imo it would be best to wait for detro/ghostdriver#511 to be merged into the official ghostdriver repository. @detro is proposing some relatively minor changes that should take me less than a week.

sorry for jumping the gun on my pull request to phantomjs.

jesg picture jesg on 27 Feb 2017

https://github.com/detro/ghostdriver/pull/511 is merged :)

henrik242 picture henrik242 on 21 Mar 2017

@Vitallium i've updated my pull request so it is based on the official ghostdriver 2.0.0 release.

jesg picture jesg on 1 Apr 2017

@jesg Nice! I guess we are ready to release 2.5

vitallium picture vitallium on 1 Apr 2017
❤5

This is good! Hope to get it in soon.

XericZephyr picture XericZephyr on 7 Apr 2017

Landed to master - https://github.com/ariya/phantomjs/commit/cad6b8852bd1ed061d4d924b3d5d7ec9386132d4

vitallium picture vitallium on 18 Jun 2017
🎉2
Was this page helpful?
0 / 5 - 0 ratings

Related issues

sinojelly picture sinojelly  路  3Comments
Snowlav picture Snowlav  路  3Comments
naitsirch picture naitsirch  路  3Comments
Sayalic0 picture Sayalic0  路  5Comments
yairza picture yairza  路  6Comments