Implemented partial link text matchfirefox and htmlunit driver

[Simon Stewart, 2008-08-14 11:43:30]

Looks like a great start. I'd rather break this into two separate checkins,
though.

http://codereview.appspot.com/2744/diff/51/113
File common/src/java/org/openqa/selenium/WebElement.java (right):

http://codereview.appspot.com/2744/diff/51/113#newcode43
Line 43: * It will not trigger a normal click action.
It should fire two click events

http://codereview.appspot.com/2744/diff/51/116
File common/src/web/doubleclickTest.html (right):

http://codereview.appspot.com/2744/diff/51/116#newcode3
Line 3: <script type="text/javascript">
Roll this into the javascript page.

http://codereview.appspot.com/2744/diff/51/115
File common/src/web/links.html (right):

http://codereview.appspot.com/2744/diff/51/115#newcode5
Line 5: <a href="">link with trailing space   </a>
And this into one of the existing pages used by the existing "By.linkText" tests

http://codereview.appspot.com/2744/diff/51/109
File common/test/java/org/openqa/selenium/AbstractDriverTestCase.java (right):

http://codereview.appspot.com/2744/diff/51/109#newcode58
Line 58: doublClickTestPage = baseUrl + "doubleclickTest.html";
There should be no need for these.

http://codereview.appspot.com/2744/diff/51/110
File common/test/java/org/openqa/selenium/DoubleClickTest.java (right):

http://codereview.appspot.com/2744/diff/51/110#newcode15
Line 15: assertTrue(driver.getTitle().equals("Hello WebDriver"));
assertEquals("Hello WebDriver", driver.getTitle());

http://codereview.appspot.com/2744/diff/51/111
File common/test/java/org/openqa/selenium/PartialLinkTextMatchTest.java (right):

http://codereview.appspot.com/2744/diff/51/111#newcode6
Line 6: public class PartialLinkTextMatchTest extends AbstractDriverTestCase {
Formatting looks strange in rietvald....

http://codereview.appspot.com/2744/diff/51/111#newcode9
Line 9: driver.findElement(By.partialLinkText("ink with leading
space")).click();
This is three different tests, and also checks the "click" functionality. Better
to do something like:

WebElement res = driver.findElement(By.partialLinkText(" with leading");
assertNotNull(res);  // This is the important one!
assertEquals("Link with leading space", res.getText()); // This confirms we got
the right element

http://codereview.appspot.com/2744/diff/51/111#newcode21
Line 21: elem = driver.findElement(By.xpath("//div"));
This test is confusing. Why do the clicking first? It would be clearer to grab
the container div, then do a sub-search in that, asserting that the correct
element was returned as done in the test above.

http://codereview.appspot.com/2744/diff/51/102
File firefox/src/java/org/openqa/selenium/firefox/FirefoxDriver.java (right):

http://codereview.appspot.com/2744/diff/51/102#newcode560
Line 560: return findElement("selectElementUsingPartialLinkText", using);
These should be grouped with the other link tests

http://codereview.appspot.com/2744/diff/51/101
File firefox/src/java/org/openqa/selenium/firefox/FirefoxWebElement.java
(right):

http://codereview.appspot.com/2744/diff/51/101#newcode237
Line 237: public WebElement findElementByPartialLinkText(String using) {
These should be grouped with the other link finders.

http://codereview.appspot.com/2744/diff/51/119
File
support/src/java/org/openqa/selenium/support/events/EventFiringWebDriver.java
(right):

http://codereview.appspot.com/2744/diff/51/119#newcode223
Line 223: element.click();
I don't think that this will do what you want it to.

[jiayao, 2008-08-27 21:44:29]

Hi Simon,
Sorry to take so long to get back to you on this feature. I've moved doubleClick
out of this CL and fixed the problems you pointed out for partial link matches.

http://codereview.appspot.com/2744/diff/51/113
File common/src/java/org/openqa/selenium/WebElement.java (right):

http://codereview.appspot.com/2744/diff/51/113#newcode43
Line 43: * It will not trigger a normal click action.
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> It should fire two click events

I guess I can do that by inspecting the element and see if the dblClick event is
handled. But the other test frameworks like htmlunit and selenium don't fall
back to two click events in this case.

http://codereview.appspot.com/2744/diff/51/115
File common/src/web/links.html (right):

http://codereview.appspot.com/2744/diff/51/115#newcode5
Line 5: <a href="">link with trailing space   </a>
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> And this into one of the existing pages used by the existing "By.linkText"
tests

Done.

http://codereview.appspot.com/2744/diff/51/109
File common/test/java/org/openqa/selenium/AbstractDriverTestCase.java (right):

http://codereview.appspot.com/2744/diff/51/109#newcode58
Line 58: doublClickTestPage = baseUrl + "doubleclickTest.html";
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> There should be no need for these.

Done.

http://codereview.appspot.com/2744/diff/51/111
File common/test/java/org/openqa/selenium/PartialLinkTextMatchTest.java (right):

http://codereview.appspot.com/2744/diff/51/111#newcode6
Line 6: public class PartialLinkTextMatchTest extends AbstractDriverTestCase {
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> Formatting looks strange in rietvald....

That was a tab, fixed.

http://codereview.appspot.com/2744/diff/51/111#newcode21
Line 21: elem = driver.findElement(By.xpath("//div"));
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> This test is confusing. Why do the clicking first? It would be clearer to grab
> the container div, then do a sub-search in that, asserting that the correct
> element was returned as done in the test above.

Done.

http://codereview.appspot.com/2744/diff/51/102
File firefox/src/java/org/openqa/selenium/firefox/FirefoxDriver.java (right):

http://codereview.appspot.com/2744/diff/51/102#newcode560
Line 560: return findElement("selectElementUsingPartialLinkText", using);
On 2008/08/14 11:43:30, simon.m.stewart wrote:
> These should be grouped with the other link tests

Done.