Commit fadf0548 authored by Dominic Battre's avatar Dominic Battre Committed by Commit Bot

More robust capturing <form>s that are taken out of the DOM.

Chrome's password manager tries to capture events where <form> elements
are taken out of the DOM or made invisible. This is used to detect
form submissions. This trigger failed in the past if a parent element
for the form was removed from the DOM or made invisible (as opposed
to the form it self). This CL makes the trigger logic more robust.

Bug: 760579, 710431
Change-Id: Idbc72d7ab57d9e6c61b0907ac5c9b3f96346d4a5
Reviewed-on: https://chromium-review.googlesource.com/892882
Commit-Queue: Dominic Battré <battre@chromium.org>
Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534037}
parent b8bfc2a4
...@@ -236,6 +236,16 @@ const char kTwoNoUsernameFormsHTML[] = ...@@ -236,6 +236,16 @@ const char kTwoNoUsernameFormsHTML[] =
" <INPUT type='submit' value='Login'/>" " <INPUT type='submit' value='Login'/>"
"</FORM>"; "</FORM>";
const char kDivWrappedFormHTML[] =
"<DIV id='outer'>"
" <DIV id='inner'>"
" <FORM id='form' action='http://www.bidule.com'>"
" <INPUT type='text' id='username'/>"
" <INPUT type='password' id='password'/>"
" </FORM>"
" </DIV>"
"</DIV>";
// Sets the "readonly" attribute of |element| to the value given by |read_only|. // Sets the "readonly" attribute of |element| to the value given by |read_only|.
void SetElementReadOnly(WebInputElement& element, bool read_only) { void SetElementReadOnly(WebInputElement& element, bool read_only) {
element.SetAttribute(WebString::FromUTF8("readonly"), element.SetAttribute(WebString::FromUTF8("readonly"),
...@@ -2536,6 +2546,56 @@ TEST_F(PasswordAutofillAgentTest, ...@@ -2536,6 +2546,56 @@ TEST_F(PasswordAutofillAgentTest,
PasswordForm::SubmissionIndicatorEvent::DOM_MUTATION_AFTER_XHR); PasswordForm::SubmissionIndicatorEvent::DOM_MUTATION_AFTER_XHR);
} }
// In this test, a <div> wrapping a form is hidden via display:none after an
// Ajax request. The test verifies that we offer to save the password, as hiding
// the <div> also hiding the <form>.
TEST_F(PasswordAutofillAgentTest, PromptForAJAXSubmitAfterHidingParentElement) {
LoadHTML(kDivWrappedFormHTML);
UpdateUsernameAndPasswordElements();
SimulateUsernameTyping("Bob");
SimulatePasswordTyping("mypassword");
FireAjaxSucceeded();
std::string hide_element =
"var outerDiv = document.getElementById('outer');"
"outerDiv.style = 'display:none';";
ExecuteJavaScriptForTests(hide_element.c_str());
base::RunLoop().RunUntilIdle();
ExpectSameDocumentNavigationWithUsernameAndPasswords(
"Bob", "mypassword", "",
PasswordForm::SubmissionIndicatorEvent::DOM_MUTATION_AFTER_XHR);
}
// In this test, a <div> wrapping a form is removed from the DOM after an Ajax
// request. The test verifies that we offer to save the password, as removing
// the <div> also removes the <form>.
TEST_F(PasswordAutofillAgentTest,
PromptForAJAXSubmitAfterDeletingParentElement) {
LoadHTML(kDivWrappedFormHTML);
UpdateUsernameAndPasswordElements();
SimulateUsernameTyping("Bob");
SimulatePasswordTyping("mypassword");
FireAjaxSucceeded();
std::string delete_element =
"var outerDiv = document.getElementById('outer');"
"var innerDiv = document.getElementById('inner');"
"outerDiv.removeChild(innerDiv);";
ExecuteJavaScriptForTests(delete_element.c_str());
base::RunLoop().RunUntilIdle();
ExpectSameDocumentNavigationWithUsernameAndPasswords(
"Bob", "mypassword", "",
PasswordForm::SubmissionIndicatorEvent::DOM_MUTATION_AFTER_XHR);
}
TEST_F(PasswordAutofillAgentTest, TEST_F(PasswordAutofillAgentTest,
NoForm_NoPromptForAJAXSubmitWithoutNavigationAndElementsVisible) { NoForm_NoPromptForAJAXSubmitWithoutNavigationAndElementsVisible) {
LoadHTML(kNoFormHTML); LoadHTML(kNoFormHTML);
......
...@@ -34,6 +34,7 @@ class WebFormElementObserverImpl::ObserverCallback ...@@ -34,6 +34,7 @@ class WebFormElementObserverImpl::ObserverCallback
private: private:
Member<HTMLElement> element_; Member<HTMLElement> element_;
HeapHashSet<Member<Node>> parents_;
Member<MutationObserver> mutation_observer_; Member<MutationObserver> mutation_observer_;
std::unique_ptr<WebFormElementObserverCallback> callback_; std::unique_ptr<WebFormElementObserverCallback> callback_;
}; };
...@@ -50,11 +51,15 @@ WebFormElementObserverImpl::ObserverCallback::ObserverCallback( ...@@ -50,11 +51,15 @@ WebFormElementObserverImpl::ObserverCallback::ObserverCallback(
init.setAttributeFilter({"action", "class", "style"}); init.setAttributeFilter({"action", "class", "style"});
mutation_observer_->observe(element_, init, ASSERT_NO_EXCEPTION); mutation_observer_->observe(element_, init, ASSERT_NO_EXCEPTION);
} }
if (element_->parentElement()) { for (Node* node = element_; node->parentElement();
node = node->parentElement()) {
MutationObserverInit init; MutationObserverInit init;
init.setChildList(true); init.setChildList(true);
mutation_observer_->observe(element_->parentElement(), init, init.setAttributes(true);
init.setAttributeFilter({"class", "style"});
mutation_observer_->observe(node->parentElement(), init,
ASSERT_NO_EXCEPTION); ASSERT_NO_EXCEPTION);
parents_.insert(node->parentElement());
} }
} }
...@@ -69,8 +74,11 @@ void WebFormElementObserverImpl::ObserverCallback::Deliver( ...@@ -69,8 +74,11 @@ void WebFormElementObserverImpl::ObserverCallback::Deliver(
for (const auto& record : records) { for (const auto& record : records) {
if (record->type() == "childList") { if (record->type() == "childList") {
for (unsigned i = 0; i < record->removedNodes()->length(); ++i) { for (unsigned i = 0; i < record->removedNodes()->length(); ++i) {
if (record->removedNodes()->item(i) != element_) Node* removed_node = record->removedNodes()->item(i);
if (removed_node != element_ &&
parents_.find(removed_node) == parents_.end()) {
continue; continue;
}
callback_->ElementWasHiddenOrRemoved(); callback_->ElementWasHiddenOrRemoved();
Disconnect(); Disconnect();
return; return;
...@@ -100,11 +108,14 @@ void WebFormElementObserverImpl::ObserverCallback::Deliver( ...@@ -100,11 +108,14 @@ void WebFormElementObserverImpl::ObserverCallback::Deliver(
void WebFormElementObserverImpl::ObserverCallback::Disconnect() { void WebFormElementObserverImpl::ObserverCallback::Disconnect() {
mutation_observer_->disconnect(); mutation_observer_->disconnect();
callback_.reset(); callback_.reset();
parents_.clear();
element_.Clear();
} }
void WebFormElementObserverImpl::ObserverCallback::Trace( void WebFormElementObserverImpl::ObserverCallback::Trace(
blink::Visitor* visitor) { blink::Visitor* visitor) {
visitor->Trace(element_); visitor->Trace(element_);
visitor->Trace(parents_);
visitor->Trace(mutation_observer_); visitor->Trace(mutation_observer_);
MutationObserver::Delegate::Trace(visitor); MutationObserver::Delegate::Trace(visitor);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment