[webkit-reviews] review granted: [Bug 51311] SegmentedString should provide column position : [Attachment 77247] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Peter Rybin
<peter.rybin at gmail.com>'s request for review:
Bug 51311: SegmentedString should provide column position
https://bugs.webkit.org/show_bug.cgi?id=51311

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=77247&action=review

Thanks for sticking with this patch.  Some minor nits below.

> WebCore/html/parser/HTMLTreeBuilder.cpp:359
> -HTMLTreeBuilder::HTMLTreeBuilder(HTMLTokenizer* tokenizer, DocumentFragment*
fragment, Element* contextElement, FragmentScriptingPermission
scriptingPermission, bool usePreHTML5ParserQuirks)
> +HTMLTreeBuilder::HTMLTreeBuilder(HTMLDocumentParser* parser, HTMLTokenizer*
tokenizer, DocumentFragment* fragment, Element* contextElement,
FragmentScriptingPermission scriptingPermission, bool usePreHTML5ParserQuirks)

This part of the change still makes me sad.  If we're going to expose the whole
HTMLDocumentParser, they we probably don't need to pass the tokenizer
separately since you can get that from the parser.  Maybe we should pass a
pointer to the input stream instead?

> WebCore/html/parser/HTMLTreeBuilder.cpp:2783
> +    ScriptableDocumentParser* parserUpcast = m_parser;
> +    TextPosition0 position = parserUpcast->textPosition();

I don't understand why wer need to do this casting.  Isn't textPosition
available on HTMLDocumentParser directory?