Bug 85353 - [chromium] Add WebKit API to access inner text value of input element
: [chromium] Add WebKit API to access inner text value of input element
Status: RESOLVED FIXED
: WebKit
Forms
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 84346 85356
  Show dependency treegraph
 
Reported: 2012-05-02 01:56 PST by
Modified: 2012-05-16 04:54 PST (History)


Attachments
Patch (2.75 KB, patch)
2012-05-02 02:01 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (2.53 KB, patch)
2012-05-06 19:25 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (9.99 KB, patch)
2012-05-11 06:58 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (10.81 KB, patch)
2012-05-15 00:15 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.60 KB, patch)
2012-05-15 01:23 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.67 KB, patch)
2012-05-15 02:43 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.82 KB, patch)
2012-05-15 05:26 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.86 KB, patch)
2012-05-15 05:41 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff
Patch (14.84 KB, patch)
2012-05-16 01:37 PST, Keishi Hattori
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-05-02 01:56:35 PST
I need this to implement the datalist UI for  <input type=email multiple>.
HTMLInputElement.value gives you the sanitized value so the whitespace between values are trimmed.
We need to append the selected suggestion to the end without modifying the rest of the text.
------- Comment #1 From 2012-05-02 02:01:43 PST -------
Created an attachment (id=139771) [details]
Patch
------- Comment #2 From 2012-05-02 02:05:08 PST -------
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
------- Comment #3 From 2012-05-02 02:09:40 PST -------
Chromium patch is here.
https://chromiumcodereview.appspot.com/10307004
------- Comment #4 From 2012-05-02 02:14:36 PST -------
(From update of attachment 139771 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=139771&action=review

> Source/WebKit/chromium/public/WebInputElement.h:78
> +        WEBKIT_EXPORT WebString innerTextValue() const;
> +        WEBKIT_EXPORT void setInnerTextValue(const WebString&);

* You should add explanation what is these functions.  It's not clear.

* "innerTextValue" is not a good name for public API. It should be visibleValue, editingValue, userEditingValue, shownValue, etc.

* If possible, we should avoid to expose setInnerTextValue().  Can you set the value by editing commands and remove setInnerTextValue()?
------- Comment #5 From 2012-05-02 03:16:12 PST -------
(In reply to comment #4)
> (From update of attachment 139771 [details] [details])
> * If possible, we should avoid to expose setInnerTextValue().  Can you set the value by editing commands and remove setInnerTextValue()?

Hmm. Using the "InsertText" command instantly reopens the autofill popup.  Calling hidePopup right after didn't hide it.

I'll see what I can do.
------- Comment #6 From 2012-05-02 04:59:32 PST -------
(In reply to comment #5)
> I'll see what I can do.

AutofillAgent was putting the textFieldDidChange event into a task queue.
I was able to work around it and achieve the original behavior by doing this. So I think I can remove setInnerTextValue.

void AutofillAgent::SetNodeText(const string16& value,
                                WebKit::WebInputElement* node) {
  string16 substring = value;
  substring = substring.substr(0, node->maxLength());

  WebKit::WebView* web_view = render_view()->GetWebView();
  if (!web_view || !web_view->focusedFrame())
    return;

  web_view->focusedFrame()->executeCommand(WebString::fromUTF8("selectAll"));
  web_view->focusedFrame()->executeCommand(WebString::fromUTF8("InsertText"), substring);
  weak_ptr_factory_.InvalidateWeakPtrs();
  MessageLoop::current()->PostTask(
                                   FROM_HERE,
                                   base::Bind(&AutofillAgent::DidSetNodeText,
                                              weak_ptr_factory_.GetWeakPtr()));
}

void AutofillAgent::DidSetNodeText() {
  WebKit::WebView* web_view = render_view()->GetWebView();
  if (web_view)
    web_view->hidePopups();
}
------- Comment #7 From 2012-05-06 19:25:28 PST -------
Created an attachment (id=140454) [details]
Patch
------- Comment #8 From 2012-05-06 19:29:54 PST -------
(In reply to comment #4)
> (From update of attachment 139771 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=139771&action=review
> 
> > Source/WebKit/chromium/public/WebInputElement.h:78
> > +        WEBKIT_EXPORT WebString innerTextValue() const;
> > +        WEBKIT_EXPORT void setInnerTextValue(const WebString&);
> 
> * You should add explanation what is these functions.  It's not clear.
Done.
> * "innerTextValue" is not a good name for public API. It should be visibleValue, editingValue, userEditingValue, shownValue, etc.
visibleValue is used in WebCore for something else so I went with editingValue.
------- Comment #9 From 2012-05-07 16:20:48 PST -------
(In reply to comment #0)
> I need this to implement the datalist UI for  <input type=email multiple>.
> HTMLInputElement.value gives you the sanitized value so the whitespace between values are trimmed.
> We need to append the selected suggestion to the end without modifying the rest of the text.

Perhaps we should expose a method to append the selected suggestion, rather than exposing the internal details?
------- Comment #10 From 2012-05-11 06:58:58 PST -------
Created an attachment (id=141409) [details]
Patch
------- Comment #11 From 2012-05-11 07:42:25 PST -------
(From update of attachment 141409 [details])
Attachment 141409 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12668419
------- Comment #12 From 2012-05-11 08:58:25 PST -------
(From update of attachment 141409 [details])
Attachment 141409 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12663724
------- Comment #13 From 2012-05-14 23:34:12 PST -------
(From update of attachment 141409 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=141409&action=review

r- because of build errors.

> Source/WebCore/html/HTMLInputElement.cpp:920
> +void HTMLInputElement::setEditingValue(const String& value)
> +{
> +    setInnerTextValue(value);
> +    subtreeHasChanged();
> +    dispatchInputEvent();
> +}

What happens if setEditingValue() is called for non-textfield input types, or textfield without a renderer?
------- Comment #14 From 2012-05-15 00:15:53 PST -------
Created an attachment (id=141874) [details]
Patch
------- Comment #15 From 2012-05-15 00:17:39 PST -------
(In reply to comment #13)
> (From update of attachment 141409 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141409&action=review
> 
> r- because of build errors.
Fixed.

> > Source/WebCore/html/HTMLInputElement.cpp:920
> > +void HTMLInputElement::setEditingValue(const String& value)
> > +{
> > +    setInnerTextValue(value);
> > +    subtreeHasChanged();
> > +    dispatchInputEvent();
> > +}
> 
> What happens if setEditingValue() is called for non-textfield input types, or textfield without a renderer?
Yes the assertion fails. Fixed.
------- Comment #16 From 2012-05-15 00:53:58 PST -------
(From update of attachment 141874 [details])
Attachment 141874 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12694532
------- Comment #17 From 2012-05-15 01:00:51 PST -------
(From update of attachment 141874 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=141874&action=review

r- because of build errors.

> Source/WebKit/chromium/public/WebInputElement.h:80
> +        // Sets the value inside the text field without being sanitized.
> +        WEBKIT_EXPORT void setEditingValue(const WebString&);

We had better mention the limitations of non-textfield types and renderer.
------- Comment #18 From 2012-05-15 01:23:40 PST -------
Created an attachment (id=141889) [details]
Patch
------- Comment #19 From 2012-05-15 02:13:19 PST -------
(From update of attachment 141889 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=141889&action=review

ok

> LayoutTests/fast/forms/editing-value.html:20
> +  testPassed("onchange event was fired.");
> +};
> +input.oninput = function() {
> +  testPassed("oninput event was fired.");

nit: We prefer 4-space indentation.

> LayoutTests/fast/forms/editing-value.html:28
> +if (window.internals)
> +    internals.setEditingValue(input, " foo ");
> +shouldBe('input.value', '"foo"');
> +shouldBe('document.querySelector(":invalid")', 'input');
> +input.blur();

We had better show a message if window.internals is not available.
------- Comment #20 From 2012-05-15 02:43:34 PST -------
Created an attachment (id=141902) [details]
Patch
------- Comment #21 From 2012-05-15 05:26:11 PST -------
Created an attachment (id=141930) [details]
Patch
------- Comment #22 From 2012-05-15 05:27:32 PST -------
Could you review this again? I changed it to move the caret to the end. (like setValue)
------- Comment #23 From 2012-05-15 05:29:31 PST -------
(From update of attachment 141930 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=141930&action=review

> Source/WebKit/chromium/public/WebInputElement.h:81
> +        // Sets the value inside the text field without being sanitized.
> +        // Can't be used if a renderer doesn't exist or on a non text field type.
> +        WEBKIT_EXPORT void setEditingValue(const WebString&);

You should mention the caret position.
------- Comment #24 From 2012-05-15 05:41:16 PST -------
Created an attachment (id=141934) [details]
Patch
------- Comment #25 From 2012-05-15 20:55:08 PST -------
(From update of attachment 141934 [details])
Rejecting attachment 141934 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 11551 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
47>At revision 11551.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/12704651
------- Comment #26 From 2012-05-16 01:37:54 PST -------
Created an attachment (id=142195) [details]
Patch
------- Comment #27 From 2012-05-16 04:54:35 PST -------
(From update of attachment 142195 [details])
Clearing flags on attachment: 142195

Committed r117263: <http://trac.webkit.org/changeset/117263>
------- Comment #28 From 2012-05-16 04:54:42 PST -------
All reviewed patches have been landed.  Closing bug.