Reported in Chrome Help Forum (Japanese): http://www.google.com/support/forum/p/chrome/thread?tid=1e819edbe70751b4&hl=ja Specification: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#dom-option-text > must return the value of the textContent IDL attribute on the element with leading and > trailing space characters removed, and with any sequences of two or more space characters > replaced by a single U+0020 SPACE character. > The space characters, for the purposes of this specification, are U+0020 SPACE, > U+0009 CHARACTER TABULATION (tab), U+000A LINE FEED (LF), U+000C FORM FEED (FF), > and U+000D CARRIAGE RETURN (CR). We use isSpaceOrNewLine() for HTMLOptionElement::text.
Created attachment 101863 [details] Patch
Comment on attachment 101863 [details] Patch 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.
(In reply to comment #2) > We already have isHTMLSpace() in WebCore/html/parserHTMLParserIdioms.h. WebCore/html/parser/HTMLParserIdioms.h.
Created attachment 101975 [details] Patch
Comment on attachment 101975 [details] Patch ok. Thank you for fixing this.
Comment on attachment 101975 [details] Patch Clearing flags on attachment: 101975 Committed r91746: <http://trac.webkit.org/changeset/91746>
All reviewed patches have been landed. Closing bug.
Did you realized if you broke the Snow Leopard build? Are you working on fixing it now?
Reopen, because it was rolled out by http://trac.webkit.org/changeset/91756
(In reply to comment #9) > Reopen, because it was rolled out by http://trac.webkit.org/changeset/91756 Hi, I fixed the build on my machine by adding the new functions to JavaScriptCore.exp: diff --git a/Source/JavaScriptCore/JavaScriptCore.exp b/Source/JavaScriptCore/JavaScriptCore.exp index 4a7a3db..d6995a6 100644 --- a/Source/JavaScriptCore/JavaScriptCore.exp +++ b/Source/JavaScriptCore/JavaScriptCore.exp @@ -584,9 +584,11 @@ __ZNK3WTF6String12toUIntStrictEPbi __ZNK3WTF6String13toInt64StrictEPbi __ZNK3WTF6String14threadsafeCopyEv __ZNK3WTF6String15stripWhiteSpaceEv +__ZNK3WTF6String15stripWhiteSpaceEPFbtE __ZNK3WTF6String16removeCharactersEPFbtE __ZNK3WTF6String17crossThreadStringEv __ZNK3WTF6String18simplifyWhiteSpaceEv +__ZNK3WTF6String18simplifyWhiteSpaceEPFbtE __ZNK3WTF6String19characterStartingAtEj __ZNK3WTF6String4utf8Eb __ZNK3WTF6String5asciiEv Regards,
rs=me to land it again with this fix. Thanks for the fix.
Created attachment 102127 [details] Patch
Comment on attachment 102127 [details] Patch ok
(In reply to comment #9) > Reopen, because it was rolled out by http://trac.webkit.org/changeset/91756 Csaba, Sorry for breaking the build. I have enhanced my workflow less likely to break the build. Anyway, I resubmit the patch which should be buildable in apple port on Snow Leopard.
Comment on attachment 102127 [details] Patch Clearing flags on attachment: 102127 Committed r91837: <http://trac.webkit.org/changeset/91837>
Comment on attachment 102127 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=102127&action=review > Source/JavaScriptCore/wtf/text/StringImpl.cpp:319 > PassRefPtr<StringImpl> StringImpl::stripWhiteSpace() > { > + return stripWhiteSpace(isSpaceOrNewline); > +} I think it will be a significant slowdown to call a function for every character, so I think this is not a good change for existing callers of this function.
(In reply to comment #17) > > PassRefPtr<StringImpl> StringImpl::stripWhiteSpace() > > { > > + return stripWhiteSpace(isSpaceOrNewline); > > +} > > I think it will be a significant slowdown to call a function for every character, so I think this is not a good change for existing callers of this function. ok, let's address this slowdown issue in Bug 65300.
*** Bug 38226 has been marked as a duplicate of this bug. ***