Chromium Code Reviews| Index: components/autofill/content/renderer/form_autofill_util.cc | 
| diff --git a/components/autofill/content/renderer/form_autofill_util.cc b/components/autofill/content/renderer/form_autofill_util.cc | 
| index 0657f78a4f64bdd7ba7056470281269ed797bb4a..123e929a211697d3245ed01e935a5c0cc935b5c9 100644 | 
| --- a/components/autofill/content/renderer/form_autofill_util.cc | 
| +++ b/components/autofill/content/renderer/form_autofill_util.cc | 
| @@ -438,6 +438,34 @@ base::string16 InferLabelForElement(const WebFormControlElement& element) { | 
| return InferLabelFromDivTable(element); | 
| } | 
| +void HandleInitiatingNode(WebFormControlElement& element, bool preview) { | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
nit: Pass-by-reference is almost entirely disallow
 
 | 
| + WebInputElement* input_element = toWebInputElement(&element); | 
| + if (!IsTextInput(input_element) && | 
| + !IsMonthInput(input_element) && | 
| + !IsTextAreaElement(element)) | 
| + return; | 
| + | 
| + if (IsTextInput(input_element) || IsMonthInput(input_element)) { | 
| + if (preview) { | 
| + input_element->setSelectionRange( | 
| + input_element->value().length(), | 
| + input_element->suggestedValue().length()); | 
| + } else { | 
| + input_element->setSelectionRange(input_element->value().length(), | 
| + input_element->value().length()); | 
| + } | 
| + } else if (IsTextAreaElement(element)) { | 
| + WebTextAreaElement text_area = element.toConst<WebTextAreaElement>(); | 
| + if (preview) { | 
| + text_area.setSelectionRange(text_area.value().length(), | 
| + text_area.suggestedValue().length()); | 
| + } else { | 
| + text_area.setSelectionRange(text_area.value().length(), | 
| + text_area.value().length()); | 
| + } | 
| + } | 
| +} | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
This method still makes it hard to tell that the c
 
 | 
| + | 
| // Fills |option_strings| with the values of the <option> elements present in | 
| // |select_element|. | 
| void GetOptionStringsFromElement(const WebSelectElement& select_element, | 
| @@ -540,8 +568,7 @@ void FillFormField(const FormFieldData& data, | 
| input_element->setValue( | 
| data.value.substr(0, input_element->maxLength()), true); | 
| if (is_initiating_node) { | 
| - int length = input_element->value().length(); | 
| - input_element->setSelectionRange(length, length); | 
| + HandleInitiatingNode(*input_element, false); | 
| // Clear the current IME composition (the underline), if there is one. | 
| input_element->document().frame()->unmarkText(); | 
| } | 
| @@ -550,6 +577,11 @@ void FillFormField(const FormFieldData& data, | 
| if (text_area.value() != data.value) { | 
| text_area.setValue(data.value); | 
| text_area.dispatchFormControlChangeEvent(); | 
| + if (is_initiating_node) { | 
| + HandleInitiatingNode(text_area, false); | 
| + // Clear the current IME composition (the underline), if there is one. | 
| + text_area.document().frame()->unmarkText(); | 
| + } | 
| } | 
| } else if (IsSelectElement(*field)) { | 
| WebSelectElement select_element = field->to<WebSelectElement>(); | 
| @@ -582,16 +614,17 @@ void PreviewFormField(const FormFieldData& data, | 
| input_element->setSuggestedValue( | 
| data.value.substr(0, input_element->maxLength())); | 
| input_element->setAutofilled(true); | 
| - if (is_initiating_node) { | 
| + if (is_initiating_node) | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
nit: Curly braces are still needed, since the body
 
 | 
| // Select the part of the text that the user didn't type. | 
| - input_element->setSelectionRange( | 
| - input_element->value().length(), | 
| - input_element->suggestedValue().length()); | 
| - } | 
| + HandleInitiatingNode(*input_element, true); | 
| + | 
| } else if (IsTextAreaElement(*field)) { | 
| WebTextAreaElement textarea = field->to<WebTextAreaElement>(); | 
| textarea.setSuggestedValue(data.value); | 
| field->setAutofilled(true); | 
| + if (is_initiating_node) | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
nit: Curly braces are still needed, since the body
 
 | 
| + // Select the part of the text that the user didn't type. | 
| + HandleInitiatingNode(textarea, true); | 
| } | 
| } | 
| @@ -784,7 +817,13 @@ void WebFormControlElementToFormField(const WebFormControlElement& element, | 
| field->text_direction = input_element->directionForFormData() == "rtl" ? | 
| base::i18n::RIGHT_TO_LEFT : base::i18n::LEFT_TO_RIGHT; | 
| } else if (IsTextAreaElement(element)) { | 
| - // Nothing more to do in this case. | 
| + const WebTextAreaElement textarea_element = | 
| + element.toConst<WebTextAreaElement>(); | 
| + field->is_autofilled = element.isAutofilled(); | 
| + field->is_focusable = element.isFocusable(); | 
| + field->should_autocomplete = textarea_element.autoComplete(); | 
| + field->text_direction = textarea_element.directionForFormData() == "rtl" ? | 
| + base::i18n::RIGHT_TO_LEFT : base::i18n::LEFT_TO_RIGHT; | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
Is it possible to refactor the code so that it is
 
 | 
| } else if (extract_mask & EXTRACT_OPTIONS) { | 
| // Set option strings on the field if available. | 
| DCHECK(IsSelectElement(element)); | 
| @@ -971,10 +1010,10 @@ bool WebFormElementToFormData( | 
| return true; | 
| } | 
| -bool FindFormAndFieldForInputElement(const WebInputElement& element, | 
| - FormData* form, | 
| - FormFieldData* field, | 
| - RequirementsMask requirements) { | 
| +bool FindFormAndFieldForFormControlElement(const WebFormControlElement& element, | 
| + FormData* form, | 
| + FormFieldData* field, | 
| + RequirementsMask requirements) { | 
| if (!IsAutofillableElement(element)) | 
| return false; | 
| @@ -992,7 +1031,7 @@ bool FindFormAndFieldForInputElement(const WebInputElement& element, | 
| field); | 
| } | 
| -void FillForm(const FormData& form, const WebInputElement& element) { | 
| +void FillForm(const FormData& form, const WebFormControlElement& element) { | 
| WebFormElement form_element = element.form(); | 
| if (form_element.isNull()) | 
| return; | 
| @@ -1033,7 +1072,7 @@ void FillFormForAllElements(const FormData& form_data, | 
| &FillFormField); | 
| } | 
| -void PreviewForm(const FormData& form, const WebInputElement& element) { | 
| +void PreviewForm(const FormData& form, const WebFormControlElement& element) { | 
| WebFormElement form_element = element.form(); | 
| if (form_element.isNull()) | 
| return; | 
| @@ -1046,7 +1085,7 @@ void PreviewForm(const FormData& form, const WebInputElement& element) { | 
| &PreviewFormField); | 
| } | 
| -bool ClearPreviewedFormWithElement(const WebInputElement& element, | 
| +bool ClearPreviewedFormWithElement(const WebFormControlElement& element, | 
| bool was_autofilled) { | 
| WebFormElement form_element = element.form(); | 
| if (form_element.isNull()) | 
| @@ -1083,26 +1122,27 @@ bool ClearPreviewedFormWithElement(const WebInputElement& element, | 
| if (IsTextInput(input_element)) { | 
| input_element->setSuggestedValue(WebString()); | 
| bool is_initiating_node = (element == *input_element); | 
| - if (is_initiating_node) | 
| + if (is_initiating_node) { | 
| input_element->setAutofilled(was_autofilled); | 
| - else | 
| + // Clearing the suggested value in the focused node (above) can cause | 
| + // selection to be lost. We force selection range to restore the text | 
| + // cursor. | 
| + HandleInitiatingNode(*input_element, false); | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
This change doesn't seem appropriate.  Is there so
 
 | 
| + } | 
| + else { | 
| 
 
Ilya Sherman
2014/02/22 06:24:20
nit: The else should be on the previous line, i.e.
 
 | 
| input_element->setAutofilled(false); | 
| - | 
| - // Clearing the suggested value in the focused node (above) can cause | 
| - // selection to be lost. We force selection range to restore the text | 
| - // cursor. | 
| - if (is_initiating_node) { | 
| - int length = input_element->value().length(); | 
| - input_element->setSelectionRange(length, length); | 
| } | 
| } else if (IsTextAreaElement(control_element)) { | 
| WebTextAreaElement text_area = control_element.to<WebTextAreaElement>(); | 
| text_area.setSuggestedValue(WebString()); | 
| bool is_initiating_node = (element == text_area); | 
| - if (is_initiating_node) | 
| + if (is_initiating_node) { | 
| control_element.setAutofilled(was_autofilled); | 
| - else | 
| + HandleInitiatingNode(text_area, false); | 
| + } | 
| + else { | 
| control_element.setAutofilled(false); | 
| + } | 
| } | 
| } | 
| @@ -1179,7 +1219,7 @@ bool IsWebElementEmpty(const blink::WebElement& element) { | 
| return true; | 
| } | 
| -gfx::RectF GetScaledBoundingBox(float scale, WebInputElement* element) { | 
| +gfx::RectF GetScaledBoundingBox(float scale, WebFormControlElement* element) { | 
| gfx::Rect bounding_box(element->boundsInViewportSpace()); | 
| return gfx::RectF(bounding_box.x() * scale, | 
| bounding_box.y() * scale, |