[webkit-reviews] review denied: [Bug 17427] Line breaking opportunities at the end of a text node are missed : [Attachment 164653] Proposed patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Alex Henrie <alexhenrie24 at gmail.com>'s
request for review:
Bug 17427: Line breaking opportunities at the end of a text node are missed
https://bugs.webkit.org/show_bug.cgi?id=17427

Attachment 164653: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=164653&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=164653&action=review


Please fix the style complaints from the style checking. They are all correctly
expressing WebKit coding style.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2446
> +		       // Thai and other Southeast Asian languages use
dictionary-based line breaking algorithms that require a lot of context

Please add a period to this sentence-style comment.

The comment is a bit mysterious. This says “a lot of context”, but does not
explain how much is enough.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2447
> +		       String textWithContext;

Just using a StringBuilder instead of a String will be significantly more
efficient. But perhaps we can do even better.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2468
> +			   current.m_nextBreakablePosition = 0; // Yes, a line
break may occur immediately before

I find this comment confusing. Maybe you could rewrite it to be a bit more
explicit?