WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
85353
[chromium] Add WebKit API to access inner text value of input element
https://bugs.webkit.org/show_bug.cgi?id=85353
Summary
[chromium] Add WebKit API to access inner text value of input element
Keishi Hattori
Reported
Wednesday, May 2, 2012 9:56:35 AM UTC
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.
Attachments
Patch
(2.75 KB, patch)
2012-05-02 02:01 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(2.53 KB, patch)
2012-05-06 19:25 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(9.99 KB, patch)
2012-05-11 06:58 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(10.81 KB, patch)
2012-05-15 00:15 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(14.60 KB, patch)
2012-05-15 01:23 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(14.67 KB, patch)
2012-05-15 02:43 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(14.82 KB, patch)
2012-05-15 05:26 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(14.86 KB, patch)
2012-05-15 05:41 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Patch
(14.84 KB, patch)
2012-05-16 01:37 PDT
,
Keishi Hattori
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Keishi Hattori
Comment 1
Wednesday, May 2, 2012 10:01:43 AM UTC
Created
attachment 139771
[details]
Patch
WebKit Review Bot
Comment 2
Wednesday, May 2, 2012 10:05:08 AM UTC
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
.
Keishi Hattori
Comment 3
Wednesday, May 2, 2012 10:09:40 AM UTC
Chromium patch is here.
https://chromiumcodereview.appspot.com/10307004
Kent Tamura
Comment 4
Wednesday, May 2, 2012 10:14:36 AM UTC
Comment on
attachment 139771
[details]
Patch 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()?
Keishi Hattori
Comment 5
Wednesday, May 2, 2012 11:16:12 AM UTC
(In reply to
comment #4
)
> (From update of
attachment 139771
[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.
Keishi Hattori
Comment 6
Wednesday, May 2, 2012 12:59:32 PM UTC
(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(); }
Keishi Hattori
Comment 7
Monday, May 7, 2012 3:25:28 AM UTC
Created
attachment 140454
[details]
Patch
Keishi Hattori
Comment 8
Monday, May 7, 2012 3:29:54 AM UTC
(In reply to
comment #4
)
> (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.
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.
Ilya Sherman
Comment 9
Tuesday, May 8, 2012 12:20:48 AM UTC
(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?
Keishi Hattori
Comment 10
Friday, May 11, 2012 2:58:58 PM UTC
Created
attachment 141409
[details]
Patch
Build Bot
Comment 11
Friday, May 11, 2012 3:42:25 PM UTC
Comment on
attachment 141409
[details]
Patch
Attachment 141409
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/12668419
Build Bot
Comment 12
Friday, May 11, 2012 4:58:25 PM UTC
Comment on
attachment 141409
[details]
Patch
Attachment 141409
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/12663724
Kent Tamura
Comment 13
Tuesday, May 15, 2012 7:34:12 AM UTC
Comment on
attachment 141409
[details]
Patch 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?
Keishi Hattori
Comment 14
Tuesday, May 15, 2012 8:15:53 AM UTC
Created
attachment 141874
[details]
Patch
Keishi Hattori
Comment 15
Tuesday, May 15, 2012 8:17:39 AM UTC
(In reply to
comment #13
)
> (From update of
attachment 141409
[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.
Build Bot
Comment 16
Tuesday, May 15, 2012 8:53:58 AM UTC
Comment on
attachment 141874
[details]
Patch
Attachment 141874
[details]
did not pass win-ews (win): Output:
http://queues.webkit.org/results/12694532
Kent Tamura
Comment 17
Tuesday, May 15, 2012 9:00:51 AM UTC
Comment on
attachment 141874
[details]
Patch 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.
Keishi Hattori
Comment 18
Tuesday, May 15, 2012 9:23:40 AM UTC
Created
attachment 141889
[details]
Patch
Kent Tamura
Comment 19
Tuesday, May 15, 2012 10:13:19 AM UTC
Comment on
attachment 141889
[details]
Patch 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.
Keishi Hattori
Comment 20
Tuesday, May 15, 2012 10:43:34 AM UTC
Created
attachment 141902
[details]
Patch
Keishi Hattori
Comment 21
Tuesday, May 15, 2012 1:26:11 PM UTC
Created
attachment 141930
[details]
Patch
Keishi Hattori
Comment 22
Tuesday, May 15, 2012 1:27:32 PM UTC
Could you review this again? I changed it to move the caret to the end. (like setValue)
Kent Tamura
Comment 23
Tuesday, May 15, 2012 1:29:31 PM UTC
Comment on
attachment 141930
[details]
Patch 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.
Keishi Hattori
Comment 24
Tuesday, May 15, 2012 1:41:16 PM UTC
Created
attachment 141934
[details]
Patch
WebKit Review Bot
Comment 25
Wednesday, May 16, 2012 4:55:08 AM UTC
Comment on
attachment 141934
[details]
Patch 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
Keishi Hattori
Comment 26
Wednesday, May 16, 2012 9:37:54 AM UTC
Created
attachment 142195
[details]
Patch
WebKit Review Bot
Comment 27
Wednesday, May 16, 2012 12:54:35 PM UTC
Comment on
attachment 142195
[details]
Patch Clearing flags on attachment: 142195 Committed
r117263
: <
http://trac.webkit.org/changeset/117263
>
WebKit Review Bot
Comment 28
Wednesday, May 16, 2012 12:54:42 PM UTC
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug