Bug 109915

Summary: Web Inspector: Several consecutive Backspace or Delete strikes should not be marked as undoable state.
Product: WebKit Reporter: Vsevolod Vlasov <vsevik>
Component: Web Inspector (Deprecated)Assignee: Vsevolod Vlasov <vsevik>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

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.