[webkit-reviews] review granted: [Bug 62971] view-source doesn't colorize </script> correctly : [Attachment 120621] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jan 3 13:20:14 PST 2012
Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 62971: view-source doesn't colorize </script> correctly
https://bugs.webkit.org/show_bug.cgi?id=62971
Attachment 120621: Patch
https://bugs.webkit.org/attachment.cgi?id=120621&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=120621&action=review
LGTM if the perf is OK.
> Source/WebCore/ChangeLog:35
> + previous segment of source. We copy them into our accumulation
> + buffer and adjust the token base offset to account for the extra
> + characters.
Did you perf test this change? Could you please mention the effect (if any) in
the ChangeLog?
> Source/WebCore/ChangeLog:56
> + - Previously, we cleared the temporary buffer lazily when we
needed
> + to add new characters to it. Now we clear it eagerly so that
it's
> + length tells us whether we're currently using it to store
> + characters.
Moar perf fear.
> Source/WebCore/html/parser/HTMLTokenizer.cpp:168
> + m_temporaryBuffer.clear();
Is temporary buffer a spec name? Seems we could give htis a more descriptive
name about what it contains or how it's used...
> Source/WebCore/html/parser/HTMLTokenizer.cpp:1590
> + characters.append('<');
> + characters.append('/');
Can we assert that we're in the "waiting for end tag state"? Seems this should
at least have a comment that the only time we buffer characters is when
searching for an end tag.
More information about the webkit-reviews
mailing list