Bug 96446

Summary:

Element::classAttributeChanged should use characters8/16 to find first non-whitespace

Product:

WebKit

Reporter:

Michael Saboff <msaboff>

Component:

DOM

Assignee:

Michael Saboff <msaboff>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cmarcelo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	benjamin: review+

Description Michael Saboff 2012-09-11 16:24:28 PDT

Element::classAttributeChanged should be changed to use characters8() or characters16() based on string bitness.

Comment 1 Michael Saboff 2012-09-11 17:43:22 PDT

Created attachment 163489 [details]
Patch

Comment 2 Benjamin Poulain 2012-09-11 18:01:36 PDT

Comment on attachment 163489 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163489&action=review

Looks like a good idea to me.

> Source/WebCore/dom/Element.cpp:765
>      for (i = 0; i < length; ++i) {
>          if (isNotHTMLSpace(characters[i]))
>              break;
>      }

If perf is important, this could become a do {} while() since we know length > 0.
Add ASSERT(length) if you do that change.

> Source/WebCore/dom/Element.cpp:773
> +    bool hasClass = !!length && (newClassString.is8Bit() ? classStringHasClassName(newClassString.characters8(), length) : classStringHasClassName(newClassString.characters16(), length));

I am not a fan of this long line.

I would change classStringHasClassName(AtomicString*) to call a classStringHasClassNameImpl<>(CharacterType, length).
That way the function can stay just as clean.

Comment 3 Michael Saboff 2012-09-12 14:40:07 PDT

Committed r128363: <http://trac.webkit.org/changeset/128363>