[Webkit-unassigned] [Bug 57746] HTMLOptionElement::text incorrectly normalize U+3000 to U+0020.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57746


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #101863|review?                     |review-
               Flag|                            |




--- Comment #2 from Kent Tamura <tkent at chromium.org>  2011-07-25 22:45:43 PST ---
(From update of attachment 101863)
View in context: https://bugs.webkit.org/attachment.cgi?id=101863&action=review

> LayoutTests/fast/forms/option-strip-unicode-spaces.html:11
> +<script src="script-tests/option-strip-unicode-spaces.js"></script>

You don't need to have a separated JavaScript file.  Please embed the JavaScript code here.

> LayoutTests/fast/forms/script-tests/option-strip-unicode-spaces.js:3
> +// see http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#space-character

Start the comment with a capital letter.
// See http:...

> LayoutTests/fast/forms/script-tests/option-strip-unicode-spaces.js:4
> +var spaces = [

The name should be 'HTMLSpaces' or something.

> LayoutTests/fast/forms/script-tests/option-strip-unicode-spaces.js:15
> +var nonSpaces = [

The name should be 'nonHTMLSpaces' or something.

> LayoutTests/fast/forms/script-tests/option-strip-unicode-spaces.js:47
> +debug('insert one space before/after/between the text');

A heading should be start with a capital letter.

> LayoutTests/fast/forms/script-tests/option-strip-unicode-spaces.js:67
> +debug('insert one non space before/after/between the text');

'one non space' is confusing.  'one non-HTML space' might be better.

> Source/WebCore/dom/OptionElement.cpp:38
> +// For removing whitespaces of text attribute in option element, only these 5 characters are treated as space.
> +// See also http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-option-text
> +static bool isOptionTextSpace(UChar c)
> +{

We already have isHTMLSpace() in WebCore/html/parserHTMLParserIdioms.h.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.