[Webkit-unassigned] [Bug 61799] Fix LayoutTests/canvas/suite/tests/2d.text.measure.width.space.html

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #95608|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #4 from Darin Adler <darin at apple.com>  2011-06-01 09:29:36 PST ---
(From update of attachment 95608)
View in context: https://bugs.webkit.org/attachment.cgi?id=95608&action=review

Wow, is the behavior really specified this way? That sounds like a mistake in the design of this API. Seems bizarre to collapse the spaces out of a string that is being passed in from JavaScript.

review- because I’d like to see a faster version of the “no collapsing needed” code path.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1879
> +    const String& str = simplifyHTMLWhiteSpace(text);

We frown on abbreviations like “str”. Maybe “simplifiedText” or the argument could be named rawText and this local could be text.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:65
> +    const UChar* fromend = from + length;

We’d normally capitalize the E here.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:66
> +    int outc = 0;

We try to avoid variable names like this with abbreviations rather than words.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:69
> +    StringBuffer data(length);

Unfortunately, this allocates a new buffer in memory every time, even if nothing changes. I think we want to further optimize the no-collapse case so the buffer is only created the first time we have to change something.

Since most strings won’t need to be collapsed, I think we could just do a complete pass checking if there is anything to collapse before actually creating the buffer.

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