MalcolmRowe + SimonStewart: Fixing up memory leaks and general nastin…

…ess in the C++ code.

r4799

CREDITS.txt

@@ -1,19 +1,19 @@
-Credits
-=======
-
-The following people have offered help, support and/or code to
-WebDriver. If you feel that you should be on this list but aren't,
-then please feel free to raise a ticket on the project site
-(http://webdriver.googlecode.com) or send an email directly to one of
-the project's maintainers.
-
-Cast
-====
-
-Joe Walnes
-Vyvyan Codd
-Zoltar - Knower of All
-Carlos Villela
-Michael Tamm
-James Cooper
-
+Credits
+=======
+
+The following people have offered help, support and/or code to
+WebDriver. If you feel that you should be on this list but aren't,
+then please feel free to raise a ticket on the project site
+(http://webdriver.googlecode.com) or send an email directly to one of
+the project's maintainers.
+
+Cast
+====
+
+Joe Walnes
+Vyvyan Codd
+Zoltar - Knower of All
+Carlos Villela
+Michael Tamm
+James Cooper
+Malcolm Rowe

common/src/web/simpleTest.html

@@ -25,13 +25,20 @@ <h1>Heading</h1>
 
 <p id="nbspandspaces">This line has a &nbsp; non-breaking space and spaces</p>
 
-<p id="inline">This <span>    line has <em>text</em>	</span> within elements that are meant to be displayed
+<p id="inline">This <span id="inlinespan">    line has <em>text</em>	</span> within elements that are meant to be displayed
     <!-- not as a block but --> inline</p>
 
 <p id="preformatted">This section has a <pre>preformatted
     text block
   within in
         </pre>
 </p>
+
+<div id="twoblocks"><p>Some text</p><p>Some more text</p></div>
+
+<div id="nestedblocks">Cheese <div><p>Some text</p><div><p>Some more text</p><p>and also</p></div></div>Brie</div>
+
+<div id="collapsingtext"><span></span><div>Hello, world</div><span></span></div>
+
 </body>
 </html>

common/test/java/com/googlecode/webdriver/SingleTestSuite.java

@@ -12,11 +12,11 @@ public static Test suite() {
 		return new TestSuiteBuilder()
 					.addSourceDir("common")
 //                    .addSourceDir("firefox")
-                    .usingDriver(FIREFOX)
+                    .usingDriver(IE)
 					.keepDriverInstance()
                     .includeJavascriptTests()
-                    .onlyRun("FrameAndWindowSwitchingTest")
-//                    .method("testShouldFireFocusKeyBlurAndChangeEventsInTheRightOrderOnIe")
+                    .onlyRun("TextHandlingTest")
+                    //.method("testHavingInlineElementsShouldNotAffectHowTextIsReturned")
 //                    .leaveRunningAfterTest()
                     .create();
 	}

common/test/java/com/googlecode/webdriver/TextHandlingTest.java

@@ -60,7 +60,7 @@ public void testShouldConvertANonBreakingSpaceIntoANormalSpaceCharacter() {
         assertThat(text, equalTo("This line has a non-breaking space"));
     }
 
-    @Ignore(value = "ie, safari", reason = "Test fails")
+    @Ignore(value = "safari", reason = "Test fails")
     public void testShouldTreatANonBreakingSpaceAsAnyOtherWhitespaceCharacterWhenCollapsingWhitespace() {
       driver.get(simpleTestPage);
       WebElement element = driver.findElement(By.id("nbspandspaces"));
@@ -135,4 +135,29 @@ public void testShouldReturnEmptyStringWhenTagIsSelfClosing() {
         String text = driver.findElement(By.id("self-closed")).getText();
         assertThat(text, equalTo(""));
     }
+
+    public void testShouldHandleSiblingBlockLevelElements() {
+    	driver.get(simpleTestPage);
+
+    	String text = driver.findElement(By.id("twoblocks")).getText();
+
+    	assertThat(text, is("Some text" + newLine + "Some more text"));
+    }
+
+    public void testShouldHandleNestedBlockLevelElements() {
+    	driver.get(simpleTestPage);
+
+    	String text = driver.findElement(By.id("nestedblocks")).getText();
+
+    	assertThat(text, is("Cheese " + newLine + "Some text" + newLine + "Some more text" + newLine + "and also" + newLine + "Brie"));
+    }
+
+    public void testShouldHandleWhitespaceInInlineElements() {
+    	driver.get(simpleTestPage);
+
+    	String text = driver.findElement(By.id("inlinespan")).getText();
+
+    	assertThat(text, is("line has text"));
+    }
+
 }

jobbie/src/cpp/InternetExplorerDriver/AbstractNode.cpp

@@ -19,51 +19,46 @@ AbstractNode::~AbstractNode()
 {
 }
 
-Node* AbstractNode::getDocument()
+Node* AbstractNode::getDocument() const
 {
-	IHTMLDOMNode2 *node2;
-	node->QueryInterface(__uuidof(IHTMLDOMNode2), (void**)&node2);
+	CComQIPtr<IHTMLDOMNode2> node2(node);
 
 	IDispatch* dispatch;
 	node2->get_ownerDocument(&dispatch);
-	node2->Release();
 
-	IHTMLDocument2 *doc;
-	dispatch->QueryInterface(__uuidof(IHTMLDocument2), (void**)&doc);
+	CComQIPtr<IHTMLDocument2> doc(dispatch);
 	dispatch->Release();
 
-	DocumentNode* toReturn = new DocumentNode(doc);
-	doc->Release();
-	return toReturn;
+	return new DocumentNode(doc);
 }
 
-Node* AbstractNode::getParent()
+Node* AbstractNode::getParent() const
 {
 	IHTMLDOMNode* parent = NULL;
 	node->get_parentNode(&parent);
 
 	if (parent == NULL)
 		return NULL;
 
-	Node* toReturn = buildNode(parent);
+	Node* toReturn = AbstractNode::buildNode(parent);
 	parent->Release();
 	return toReturn;
 }
 
-Node* AbstractNode::getFirstChild()
+Node* AbstractNode::getFirstChild() const
 {
 	IHTMLDOMNode* child = NULL;
 	node->get_firstChild(&child);
 
 	if (child == NULL)
 		return NULL;
 
-	Node* toReturn = buildNode(child);
+	Node* toReturn = AbstractNode::buildNode(child);
 	child->Release();
 	return toReturn;
 }
 
-Node* AbstractNode::getNextSibling() 
+Node* AbstractNode::getNextSibling() const
 {
 	IHTMLDOMNode* sibling = NULL;
 	node->get_nextSibling(&sibling);
@@ -72,39 +67,33 @@ Node* AbstractNode::getNextSibling()
 		return NULL;
 	}
 
-	Node* toReturn = buildNode(sibling);
+	Node* toReturn = AbstractNode::buildNode(sibling);
 	sibling->Release();
 	return toReturn;
 }
 
-const std::wstring AbstractNode::name()
+std::wstring AbstractNode::name() const
 {
-	BSTR name;
+	CComBSTR name;
 	node->get_nodeName(&name);
-	const std::wstring toReturn = bstr2wstring(name);
-	SysFreeString(name);
-	return toReturn;
+	return bstr2wstring(name);
 }
 
-const wchar_t* AbstractNode::getText()
+std::wstring AbstractNode::getText() const
 {
-	IHTMLElement* element;
-	node->QueryInterface(__uuidof(element), (void**)&element);
+	CComQIPtr<IHTMLElement> element(node);
 
-	BSTR text;
+	CComBSTR text;
 	element->get_innerText(&text);
-	element->Release();
-	const wchar_t* toReturn = bstr2wchar(text);
-	SysFreeString(text);
-	return toReturn;
+	return bstr2wstring(text);
 }
 
-IHTMLDOMNode* AbstractNode::getDomNode()
+IHTMLDOMNode* AbstractNode::getDomNode() const
 {
 	return node;
 }
 
-Node* AbstractNode::buildNode(IHTMLDOMNode* from) 
+Node* AbstractNode::buildNode(IHTMLDOMNode *from) 
 {
 	long type = 0;
 	from->get_nodeType(&type);

jobbie/src/cpp/InternetExplorerDriver/AbstractNode.h

@@ -9,20 +9,20 @@ class AbstractNode : public Node
 	AbstractNode(IHTMLDOMNode*);
 	virtual ~AbstractNode(void);
 
-	virtual Node* getDocument();
-	virtual Node* getNextSibling();
-	virtual Node* getFirstChild();
-	virtual Node* getParent();
-	virtual Node* getFirstAttribute() = 0;
+	virtual Node* getDocument() const;
+	virtual Node* getNextSibling() const;
+	virtual Node* getFirstChild() const;
+	virtual Node* getParent() const;
+	virtual Node* getFirstAttribute() const = 0;
 
-	virtual const std::wstring name();
-	virtual const wchar_t* getText();
+	virtual std::wstring name() const;
+	virtual std::wstring getText() const;
 
-	IHTMLDOMNode* getDomNode();
+	IHTMLDOMNode* getDomNode() const;
 
 protected:
 
-	Node* buildNode(IHTMLDOMNode*);
+	static Node* buildNode(IHTMLDOMNode *from);
 
 	CComPtr<IHTMLDOMNode> node;
 };

jobbie/src/cpp/InternetExplorerDriver/AttributeNode.cpp

@@ -8,31 +8,26 @@
 
 using namespace std;
 
-AttributeNode::AttributeNode(IEnumVARIANT* enumerator)
+AttributeNode::AttributeNode(IEnumVARIANT* allAttributes)
+	: enumerator(allAttributes), attribute(NULL)
 {
-	this->enumerator = enumerator;
-	this->enumerator->AddRef();
-
 	moveToNextSpecifiedIndex();
 
 	if (this->attribute == NULL) {
-		this->enumerator->Release();
 		throw "No declared attributes";
 	}
 }
 
 AttributeNode::~AttributeNode()
 {
-	attribute->Release();
-	enumerator->Release();
 }
 
-Node* AttributeNode::getDocument() 
+Node* AttributeNode::getDocument() const
 {
 	return NULL;
 }
 
-Node* AttributeNode::getNextSibling()
+Node* AttributeNode::getNextSibling() const
 {
 	try {
 		return new AttributeNode(enumerator);
@@ -41,17 +36,17 @@ Node* AttributeNode::getNextSibling()
 	}
 }
 
-Node* AttributeNode::getFirstChild() 
+Node* AttributeNode::getFirstChild() const
 {
 	return NULL;
 }
 
-Node* AttributeNode::getFirstAttribute() 
+Node* AttributeNode::getFirstAttribute() const
 {
 	return NULL;
 }
 
-const std::wstring AttributeNode::name()
+std::wstring AttributeNode::name() const
 {
 	BSTR name;
 	attribute->get_nodeName(&name);
@@ -65,11 +60,11 @@ const std::wstring AttributeNode::name()
 	return toReturn;
 }
 
-const wchar_t* AttributeNode::getText()
+std::wstring AttributeNode::getText() const
 {
 	VARIANT value;
 	attribute->get_nodeValue(&value);
-	const wchar_t* toReturn = variant2wchar(value);
+	std::wstring toReturn = variant2wchar(value);
 	VariantClear(&value);
 	return toReturn;
 }
@@ -85,15 +80,13 @@ void AttributeNode::moveToNextSpecifiedIndex()
 		if (nextAttribute == NULL)
 			return;
 
-		IHTMLDOMAttribute* attr;
-		nextAttribute->QueryInterface(__uuidof(IHTMLDOMAttribute), (void**)&attr);
+		CComQIPtr<IHTMLDOMAttribute> attr(nextAttribute);
 
 		VARIANT_BOOL specified;
 		attr->get_specified(&specified);
 		if (specified == VARIANT_TRUE) {
 			this->attribute = attr;
 			return;
 		}
-		attr->Release();
 	}
 }

jobbie/src/cpp/InternetExplorerDriver/AttributeNode.h

@@ -10,17 +10,17 @@ class AttributeNode : public Node
 	AttributeNode(IEnumVARIANT* enumerator);
 	~AttributeNode();
 
-	Node* getDocument();
-	Node* getNextSibling();
-	Node* getFirstChild();
-	Node* getFirstAttribute();
+	virtual Node* getDocument() const;
+	virtual Node* getNextSibling() const;
+	virtual Node* getFirstChild() const;
+	virtual Node* getFirstAttribute() const;
 
-	const std::wstring name();
-	const wchar_t* getText();
+	std::wstring name() const;
+	std::wstring getText() const;
 
 private:
 	void moveToNextSpecifiedIndex();
 
-	IEnumVARIANT* enumerator;
-	IHTMLDOMAttribute* attribute;
+	CComPtr<IEnumVARIANT> enumerator;
+	CComPtr<IHTMLDOMAttribute> attribute;
 };