Bug 109915 - Web Inspector: Several consecutive Backspace or Delete strikes should not be marked as undoable state.
Summary: Web Inspector: Several consecutive Backspace or Delete strikes should not be ...
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: Vsevolod Vlasov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-15 02:07 PST by Vsevolod Vlasov
Modified: 2013-02-15 08:55 PST (History)
9 users (show)

See Also:


Attachments
Patch (15.01 KB, patch)
2013-02-15 02:15 PST, Vsevolod Vlasov
no flags Details | Formatted Diff | Diff
Patch (15.34 KB, patch)
2013-02-15 07:57 PST, Vsevolod Vlasov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vsevolod Vlasov 2013-02-15 02:07:40 PST
Patch to follow.
Comment 1 Vsevolod Vlasov 2013-02-15 02:15:30 PST
Created attachment 188517 [details]
Patch
Comment 2 Pavel Feldman 2013-02-15 03:37:39 PST
Comment on attachment 188517 [details]
Patch

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

> Source/WebCore/inspector/front-end/TextEditorModel.js:288
> +            if (this._lastCommand.newRange.endLine === range.startLine && this._lastCommand.newRange.endColumn === range.startColumn)

This looks like a utility method (TextRange.prototypes.isSiblingRange)

> Source/WebCore/inspector/front-end/TextEditorModel.js:298
> +        if (text.length > 1 || text === "\n")

Due to the editRangeLogic, even single key press can result in a text.length > 1 (and I can imagine coalescing events that are converted into single editRange). Could we simplify this whole heuristics?
Comment 3 Pavel Feldman 2013-02-15 03:55:15 PST
Comment on attachment 188517 [details]
Patch

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

> Source/WebCore/inspector/front-end/TextEditorModel.js:256
> +    _rangeHasOneCharacter: function(range)

Move to text range prototype.
Comment 4 Vsevolod Vlasov 2013-02-15 07:57:09 PST
Created attachment 188572 [details]
Patch
Comment 5 Vsevolod Vlasov 2013-02-15 08:00:46 PST
(In reply to comment #3)
> (From update of attachment 188517 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188517&action=review
> 
> > Source/WebCore/inspector/front-end/TextEditorModel.js:256
> > +    _rangeHasOneCharacter: function(range)
> 
> Move to text range prototype.

It's not really possible since it depends on the line length.
Comment 6 Pavel Feldman 2013-02-15 08:35:42 PST
Comment on attachment 188572 [details]
Patch

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

> Source/WebCore/inspector/front-end/TextEditorModel.js:278
> +    _rangeHasOneCharacter: function(range)

This could be even public :)
Comment 7 WebKit Review Bot 2013-02-15 08:55:36 PST
Comment on attachment 188572 [details]
Patch

Clearing flags on attachment: 188572

Committed r143005: <http://trac.webkit.org/changeset/143005>
Comment 8 WebKit Review Bot 2013-02-15 08:55:40 PST
All reviewed patches have been landed.  Closing bug.