Bug 62061 - Input value sanitization for text fields is incorrect
Summary: Input value sanitization for text fields is incorrect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 37024 41115
  Show dependency tree / graph
 
Reported: 2011-06-03 15:46 PDT by Alexey Proskuryakov
Modified: 2011-06-04 04:23 PDT (History)
2 users (show)

See Also:

Attachments
proposed fix (8.01 KB, patch)
2011-06-03 17:48 PDT, Alexey Proskuryakov 		darin: review+
Details | Formatted Diff | Diff
patch for landing (7.98 KB, patch)
2011-06-03 21:46 PDT, Alexey Proskuryakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2011-06-03 15:46:29 PDT 
The spec says that newlines should be stripped, not replaced with spaces.

<rdar://problem/9553273>
Comment 1 Alexey Proskuryakov 2011-06-03 15:46:49 PDT 
HTML5 parser relies on value sanitization for correct behavior in this case:

<input type="text" placeholder="foo" value="
">

In Safari 5, the placeholder is displayed, and that's correct per HTML5. But it doesn't work in ToT - and besides, the linefeed appears as whitespace in initial value, which users are unlikely to notice when typing.
Comment 2 Alexey Proskuryakov 2011-06-03 17:48:21 PDT 
Created attachment 95998 [details]
proposed fix
Comment 3 Darin Adler 2011-06-03 18:09:08 PDT 
Comment on attachment 95998 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=95998&action=review

> Source/WebCore/html/TextFieldInputType.cpp:189
> +static bool asciiLineBreakCharacters(UChar c)

The name should be something more like:

    isASCIILineBreak
Comment 4 Alexey Proskuryakov 2011-06-03 21:46:04 PDT 
I didn't really expect that would you fly, but I liked how removeCharacters(asciiLineBreakCharacters) looked.

Thanks for the review!
Comment 5 Alexey Proskuryakov 2011-06-03 21:46:40 PDT 
Created attachment 96006 [details]
patch for landing
Comment 6 Alexey Proskuryakov 2011-06-04 00:21:20 PDT 
_ that_it_would_fly_ :-/
Comment 7 WebKit Review Bot 2011-06-04 04:23:22 PDT 
Comment on attachment 96006 [details]
patch for landing

Clearing flags on attachment: 96006

Committed r88110: <http://trac.webkit.org/changeset/88110>
Comment 8 WebKit Review Bot 2011-06-04 04:23:26 PDT 
All reviewed patches have been landed.  Closing bug.