Bug 102688 - Web Inspector: simplify? damaged region computation in the editor
Summary: Web Inspector: simplify? damaged region computation in the editor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-19 06:58 PST by Pavel Feldman
Modified: 2012-11-21 22:39 PST (History)
8 users (show)

See Also:


Attachments
Patch (27.32 KB, patch)
2012-11-19 06:59 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (28.82 KB, patch)
2012-11-19 22:49 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (28.71 KB, patch)
2012-11-19 23:03 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (29.28 KB, patch)
2012-11-20 07:04 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (29.42 KB, patch)
2012-11-20 07:56 PST, Pavel Feldman
vsevik: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2012-11-19 06:58:02 PST
This change simplifies the damaged region computation. Drive-by make _getSelection work properly.
Comment 1 Pavel Feldman 2012-11-19 06:59:33 PST
Created attachment 174972 [details]
Patch
Comment 2 Pavel Feldman 2012-11-19 22:49:21 PST
Created attachment 175144 [details]
Patch
Comment 3 Pavel Feldman 2012-11-19 23:03:16 PST
Created attachment 175147 [details]
Patch
Comment 4 Vsevolod Vlasov 2012-11-20 05:21:13 PST
Comment on attachment 175147 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1891
> +            node = this._enclosingLineRowOrSelf(container);

This if branch should precede previous one. We should make sure that the enclosing line row of container is preceding lastUndamaged line row.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2128
> +        var lastUndamagedChunk = lastUndamagedLineNumber < this._textModel.linesCount ? this._textChunks[this._chunkNumberForLine(lastUndamagedLineNumber)] : null;

lastUndamagedLineChunk

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2129
> +        var firstUndamagedChunk = firstUndamagedLineNumber >= 0 ? this._textChunks[this._chunkNumberForLine(firstUndamagedLineNumber)] : null;

I think you should invert checks for these two lines.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2131
> +        var collectLinesFromNode = lastUndamagedChunk ? lastUndamagedChunk.lineRowContainingLine(lastUndamagedLineNumber) : null;

lastUndamagedLineRow

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2202
> +        var cloneToLine = Math.min(Math.max(endLine, range.endLine) + lineCountDelta + 1, this._textModel.linesCount);

replace endLine  with startLine + lines.length.

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:2681
> +WebInspector.debugDefaultTextEditor = true;

Please remove
Comment 5 Pavel Feldman 2012-11-20 07:04:43 PST
Created attachment 175211 [details]
Patch
Comment 6 Pavel Feldman 2012-11-20 07:56:35 PST
Created attachment 175221 [details]
Patch
Comment 7 Pavel Feldman 2012-11-20 08:32:32 PST
Committed r135285: <http://trac.webkit.org/changeset/135285>
Comment 8 eustas.bug 2012-11-21 22:39:13 PST
Comment on attachment 175221 [details]
Patch

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

> Source/WebCore/inspector/front-end/DefaultTextEditor.js:1989
> +        if (false) // For paint debugging.

This makes compiler unhappy: "WARNING - unreachable code"