Bug 101004

Summary:

quoteCSSString() always creates a 16 bit string

Product:

WebKit

Reporter:

Michael Saboff <msaboff>

Component:

CSS

Assignee:

Michael Saboff <msaboff>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cmarcelo, dglazkov, macpherson, menard, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	darin: review+, webkit.review.bot: commit-queue-
Patch with suggested changes and speculative chromium fix	none

Michael Saboff

Reported 2012-11-01 18:27:31 PDT

quoteCSSString() in CSSParser.cpp should create an 8 bit string for most case.

Attachments
Patch (3.70 KB, patch) 2012-11-01 18:36 PDT, Michael Saboff	darin: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch with suggested changes and speculative chromium fix (3.80 KB, patch) 2012-11-02 15:23 PDT, Michael Saboff	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Michael Saboff

Comment 1 2012-11-01 18:36:31 PDT

Created attachment 171967 [details] Patch

Darin Adler

Comment 2 2012-11-02 10:56:21 PDT

Comment on attachment 171967 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171967&action=review > Source/WebCore/css/CSSParser.cpp:10640 > + if (string.length() >= (std::numeric_limits<unsigned>::max() / 3) - 2) Not new code, but seems to me there are extra parentheses here. Also seems strange to use >= here instead of just >. > Source/WebCore/css/CSSParser.cpp:10641 > + return ""; Should probably use emptyString() here since it makes more efficient code.

WebKit Review Bot

Comment 3 2012-11-02 14:36:07 PDT

Comment on attachment 171967 [details] Patch Attachment 171967 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14670963 New failing tests: svg/W3C-SVG-1.1/animate-elem-78-t.svg svg/W3C-SVG-1.1/animate-elem-52-t.svg

Michael Saboff

Comment 4 2012-11-02 15:23:43 PDT

Created attachment 172157 [details] Patch with suggested changes and speculative chromium fix

Darin Adler

Comment 5 2012-11-06 09:03:40 PST

Comment on attachment 172157 [details] Patch with suggested changes and speculative chromium fix I wonder if it would be better to change this to use StringBuilder instead of StringBuffer at some point.

WebKit Review Bot

Comment 6 2012-11-06 09:44:43 PST

Comment on attachment 172157 [details] Patch with suggested changes and speculative chromium fix Clearing flags on attachment: 172157 Committed r133625: <http://trac.webkit.org/changeset/133625>

WebKit Review Bot

Comment 7 2012-11-06 09:44:46 PST

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.