Summary: | createElement should not lowercase non-ASCII characters | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||
Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, esprehn+autocc, gyuyoung.kim, kangil.han, kling, koivisto, sam | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 150225 | ||||||||
Attachments: |
|
Description
Ryosuke Niwa
2016-01-14 19:23:16 PST
Created attachment 269025 [details]
Fixes the bug
Created attachment 269026 [details]
Add a reference to the spec in the change log
Comment on attachment 269026 [details] Add a reference to the spec in the change log View in context: https://bugs.webkit.org/attachment.cgi?id=269026&action=review > Source/WebCore/dom/Document.h:384 > + WEBCORE_EXPORT RefPtr<Element> createElement(const AtomicString& tagName, ExceptionCode&); There's no new use of this outside of WebCore. I don't think WEBCORE_EXPORT is needed here. (In reply to comment #3) > Comment on attachment 269026 [details] > Add a reference to the spec in the change log > > View in context: > https://bugs.webkit.org/attachment.cgi?id=269026&action=review > > > Source/WebCore/dom/Document.h:384 > > + WEBCORE_EXPORT RefPtr<Element> createElement(const AtomicString& tagName, ExceptionCode&); > > There's no new use of this outside of WebCore. I don't think WEBCORE_EXPORT > is needed here. It's used by WKDOMDocument.mm in WebKit2. Comment on attachment 269026 [details] Add a reference to the spec in the change log Clearing flags on attachment: 269026 Committed r195091: <http://trac.webkit.org/changeset/195091> All reviewed patches have been landed. Closing bug. Comment on attachment 269026 [details] Add a reference to the spec in the change log View in context: https://bugs.webkit.org/attachment.cgi?id=269026&action=review > Source/WebCore/html/HTMLDocument.cpp:-235 > - return HTMLElementFactory::createElement(QualifiedName(nullAtom, name.lower(), xhtmlNamespaceURI), *this); Aside from the fix for this specific bug, it’s worth having someone audit WebKit code to eliminate most uses of this function, and possibly renaming it to a explicit longer name so it’s not attractive for people writing new code. There are almost no cases where String::lower, String::upper, String::foldCase, or the StringImpl versions are the best functions to call. The few cases where these are needed are cases where we are processing actual document or user-typed text content, such as in the implementation of text-transform and of searching, and in those cases we almost always need to do something ore complicated than simply processing a single string without taking locale into account. In the vast majority of cases where we do use the functions, we are simply trying to process things in a way that is not sensitive to *ASCII* case because that concept arises in many places in standards like HTML, HTTP, DNS, etc. When we use these Unicode case processing functions we either get incorrect behavior (as we did here), or correct behavior but with more complex code doing more work that is necessary for no benefit. There’s a similar issue with the stripWhiteSpace and simplifyWhiteSpace functions. |