[Webkit-unassigned] [Bug 45271] HTML parser should provide script column position within HTML document to JavaScript engine

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45271





--- Comment #27 from Peter Rybin <peter.rybin at gmail.com>  2010-09-24 14:02:53 PST ---
(From update of attachment 68715)
View in context: https://bugs.webkit.org/attachment.cgi?id=68715&action=review

Hi Adam.
I agree on simplifying the patch. The 'generation' field doesn't seem extremely elegant, however I need some way of telling that the script does not belong to original text (i.e. it is generated). It's not necessarily a position property, but at least it's something very close.

The only problem I'm still having is making "build-webkit --qt" use XmlDocumentParserQt.cc rather that *Libxml2.cc. That's why I've uploaded bad patches so often.
Peter

>> JavaScriptCore/GNUmakefile.am:517
>> +	JavaScriptCore/wtf/TextPosition.h \
> 
> These should probably be in JavaScriptCore/wtf/text

Done

>> JavaScriptCore/wtf/TextPosition.h:39
>> + * call. While elements of the generated part may still have coordinates (probably),
> 
> I'm not sure generation is appropriate for WTF.  WTF doesn't know anything about document.write.

Done

>> JavaScriptCore/wtf/TextPosition.h:86
>> +    int m_generation;
> 
> This doesn't seem right here.

Done

>> WebCore/bindings/js/ScriptSourceCode.h:45
>> +    ScriptSourceCode(const String& source, const KURL& url = KURL(), const TextPosition1& startPosition = TextPosition1::minimumPosition())
> 
> TextPosition1 => Does this build?

Initializing const reference with a function call? Apparently it does.

>> WebCore/bindings/v8/ScriptEventListener.cpp:54
>> +    TextPosition0 position(WTF::ZeroBasedNumber::fromZeroBasedInt(1), WTF::ZeroBasedNumber::s_base, 0);
> 
> Yes, I think this code is wrong.  :)
> 
> Let's fix it in another patch.

Done

>> WebCore/dom/XMLDocumentParserLibxml2.cpp:1373
>> +    //   New "- 1" fixes this. Is it correct?
> 
> I'm confused.  Are there tests that break with or without the -1?  We need tests to cover this.  If there are tests, we can skip the comment.  If there are no tests, I'd leave the code as-is, can keep the first line of the comment (with a capital "T").  We can then write the test and fix the bug in a followup patch.

Done #2

>> WebCore/dom/XMLDocumentParserLibxml2.cpp:1381
>> +    return  (context() ? context()->input->col : 1) - 1;
> 
> Same here.

Done #2

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.