Bug 41691 – Web Inspector: provide line numbers for inline styles.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Pavel Feldman]

Patch to follow.

[Pavel Feldman]

Created attachment 60631 [details]
[PATCH] Proposed change.

We will probably need Dave's blessing for this one, but please review it before I add him to the CC list.

[Pavel Feldman]

Comments for the patch: In order to provide rules of inline css with proper line numbers, I get current line number from the parser upon HTMLStyleElement creation (I store it in int there). Then I extend StyleElement's interface to accept the line number baseline that is used in the CSSParser when constructing rules.

[Joseph Pecoraro]

Comment on attachment 60631 [details]
[PATCH] Proposed change.

> +        * inspector/styles-source-lines-expected.txt: Removed.
> +        * inspector/styles-source-lines.html:

This test is still here, but probably changed. Should there be
results checked in for it, or do you want those generated?


>  bool CSSStyleSheet::parseString(const String &string, bool strict)
>  {
> +    return parseStringAtLine(string, strict, 0);
> +}
...
>      virtual bool parseString(const String&, bool strict = true);

Can this be inlined? Are compilers smart enough to do it anyways?


> +    int m_startLine;

My comment would have been to make these counters "unsigned". But
there are already plenty of line counters as "int". This is fine then.

Looks good to me (with those updated test results)!

[Pavel Feldman]

(In reply to comment #2)
> Comments for the patch: In order to provide rules of inline css with proper line numbers, I get current line number from the parser upon HTMLStyleElement creation (I store it in int there). Then I extend StyleElement's interface to accept the line number baseline that is used in the CSSParser when constructing rules.

Dave, could you take a look at it? It adds an argument to StyleElement and stores int in HTMLStyleElement.

[Joseph Pecoraro]

Comment on attachment 60631 [details]
[PATCH] Proposed change.

> +++ b/WebCore/html/HTMLStyleElement.cpp
> @@ -37,7 +38,10 @@ inline HTMLStyleElement::HTMLStyleElement(const QualifiedName& tagName, Document
>      : HTMLElement(tagName, document)
>      , m_loading(false)
>      , m_createdByParser(createdByParser)
> +    , m_startLine(0)
>  {
> +    if (createdByParser && document && document->scriptableDocumentParser())
> +        m_startLine = document->scriptableDocumentParser()->lineNumber();
>      ASSERT(hasTagName(styleTag));
>  }

One last thing, move this after the ASSERT.
An extra int per style element should be fine.
r=me

[Pavel Feldman]

Landed with review comments addressed. Also renamed startLine to startLineNumber:

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/http/tests/inspector/inspector-test.js
	M	LayoutTests/inspector/elements-tests.js
	A	LayoutTests/inspector/resources/styles-source-lines-inline-iframe.html
	M	LayoutTests/inspector/styles-iframe.html
	M	LayoutTests/inspector/styles-source-lines-expected.txt
	A	LayoutTests/inspector/styles-source-lines-inline-expected.txt
	A	LayoutTests/inspector/styles-source-lines-inline.html
	M	LayoutTests/inspector/styles-source-lines.html
	M	WebCore/ChangeLog
	M	WebCore/css/CSSParser.cpp
	M	WebCore/css/CSSParser.h
	M	WebCore/css/CSSStyleSheet.cpp
	M	WebCore/css/CSSStyleSheet.h
	M	WebCore/dom/StyleElement.cpp
	M	WebCore/dom/StyleElement.h
	M	WebCore/html/HTMLStyleElement.cpp
	M	WebCore/html/HTMLStyleElement.h
	M	WebCore/inspector/InspectorCSSStore.cpp
	M	WebCore/inspector/InspectorDOMAgent.cpp
	M	WebCore/inspector/front-end/DOMAgent.js
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/svg/SVGStyleElement.cpp
Committed r62630