[webkit-reviews] review granted: [Bug 122694] Factor line box code from RenderText to a class : [Attachment 214057] patch

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 122694: Factor line box code from RenderText to a class
https://bugs.webkit.org/show_bug.cgi?id=122694

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

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=214057&action=review


> Source/WebCore/rendering/RenderTextLineBoxes.cpp:43
> +	   m_first = m_last = textBox;

We should do this on two lines.

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:59
> +	   m_first = 0;

nullptr

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:114
> +    m_first = m_last = 0;

nullptr
Also, we should do this on two lines.

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:117
> +InlineTextBox* RenderTextLineBoxes::findNext(int offset, int& pos) const

Crappy name: pos.

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:120
> +	   return 0;

nullptr

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:129
> +    pos = (offset > currentOffset ? current->len() : current->len() -
(currentOffset - offset) );

Extra space before last ')'

> Source/WebCore/rendering/RenderTextLineBoxes.cpp:143
> +    const InlineTextBox* prev = 0;

nullptr

> Source/WebCore/rendering/RenderTextLineBoxes.h:45
> +    void extract(InlineTextBox*);
> +    void attach(InlineTextBox*);
> +    void remove(InlineTextBox*);

Seems like these should really take references. Maybe in a later patch?