-
Notifications
You must be signed in to change notification settings - Fork 130
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
Upgrade htmlunit to 2.36.0 #171
Conversation
* @param domNode the node to start searching from | ||
* @param xpathExpr the XPath expression | ||
* @return the list of objects found. | ||
*/ | ||
public static <E> List<E> selectNodes(final DomNode domNode, final String xpathExpr) { | ||
WebClientUtil.waitForJSExec(domNode.getPage().getWebClient()); | ||
return (List) XPathUtils.getByXPath(domNode, xpathExpr, null); | ||
return (List) XPathHelper.getByXPath(domNode, xpathExpr, null, true); |
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.
I am fine with that, but I would rather like to release the staged fixes first. HTMLUnit always causes regressions in downstream test suites
@Override | ||
public void warn(String s, String s1, int i, String s2, int i1) { | ||
|
||
} |
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 think it would make sense to use the implementation in https://github.com/jenkinsci/jenkins-test-harness/pull/179/files so the change matches better with the current coding style: no (meaningless) comment, correct parameter names, no empty line.
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.
take whichever, they should be pretty much the same PR 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.
I think it is ready to go. It will likely cause compatibility issues in the downstream (as always), but we are kinda used to that
diff HtmlUnit/htmlunit@HtmlUnit-2.31...2.36.0