Bug 54876 – Web Inspector: [Text editor] Bug in the highlighter

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

[Andrey Adaikin]

There is a bug in highlighter. Here is an example:
=====
1 /*
2 
3 1
4 2
5 3
6 4
7 5
8 
9 */
=====
Copy paste lines #3-#7 inclusive onto the line #8 => the highlights will be broken after the line #8.

[Andrey Adaikin]

Created attachment 83155 [details]
Patch

[Pavel Feldman]

Comment on attachment 83155 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83155&action=review

> LayoutTests/inspector/editor/highlighter-bugs-test-expected.txt:3
> +doTestForBug1()

Bug1?

> LayoutTests/inspector/editor/highlighter-bugs-test-expected.txt:7
> +    0 : {

Could you use a more dense format? (Look at syntax-highlight-* tests for inspiration).

> LayoutTests/inspector/editor/highlighter-bugs-test.html:14
> +    window.setTimeout = function(func) {

Bad things may happen. You should either use sniffers (InspectorTest.addSniffer) or extend highlighter API (add "immediately" optional parameter) to workaround asynchronous operations.

[Andrey Adaikin]

Created attachment 83302 [details]
Patch

[Andrey Adaikin]

There is also a regression bug in the highlighter for long lines. I fixed it also in this CL, and added another test.

[Pavel Feldman]

Comment on attachment 83302 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83302&action=review

This looks good. r- for timeout, as we agreed.

> LayoutTests/inspector/editor/highlighter-long-line.html:30
> +        function() {

"{" on the next line.

function()
{
}

> LayoutTests/inspector/editor/highlighter-long-line.html:47
> +        function() {

ditto

> LayoutTests/inspector/editor/highlighter-paste-in-comment.html:24
> +        function() {

ditto (and below)

> LayoutTests/inspector/editor/highlighter-test.js:22
> +InspectorTest.waitForAllTimeouts = function(runnable, callback)

As we agreed, let's change the source code API for better testability.

[Andrey Adaikin]

Created attachment 83632 [details]
Patch

[Pavel Feldman]

Comment on attachment 83632 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83632&action=review

Otherwise looks good.

> LayoutTests/ChangeLog:5
> +        Web Inspector: [Text editor] Bug in the highlighter

Need more details on the issue in the change log.

[Andrey Adaikin]

Created attachment 83644 [details]
Patch

[Pavel Podivilov]

Comment on attachment 83644 [details]
Patch

Clearing flags on attachment: 83644

Committed r79566: <http://trac.webkit.org/changeset/79566>

[Pavel Podivilov]

All reviewed patches have been landed.  Closing bug.