Bug 26602

Summary: Web Inspector: caret moves past prompt in javascript console
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, commit-queue, joepeck, justin.garcia, timothy
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed solution
none
[PATCH] Manual removal of empty Text nodes
pfeldman: review+
[PATCH] Comments addressed, explanation added to ChangeLog none

Description Pavel Feldman 2009-06-22 02:47:56 PDT
1. Open javascript console
2. Start typing "document" until auto-complete shows up, but don't finish the word
3. Hold backspace

What is the expected output? What do you see instead?
The caret should stop at the prompt character ">"
I'm seeing the caret jump past the prompt character.
If you start typing it goes back to normal, but it's a little strange!

(upstreaming http://code.google.com/p/chromium/issues/detail?id=14710).
Comment 1 Joseph Pecoraro 2009-08-07 08:05:44 PDT
The style on the console-prompt is:

  -webkit-user-modify: read-write-plaintext-only;
  min-height: 16px;
  padding: 1px 22px 1px 24px;
  position: relative;
  white-space: pre-wrap;

So there is always 24px of padding on the left.  I'm guessing that the proprietary "-webkit-user-modify: read-write-plaintext-only" could be causing the problem by incorrectly working with backspace and padding? Attempting to change to margin-left didn't work either.
Comment 2 Timothy Hatcher 2009-08-07 14:41:20 PDT
This smells like an editing bug.

I actually hit this today doing a demo.
Comment 3 Alexander Pavlov (apavlov) 2010-03-18 07:02:56 PDT
Created attachment 51022 [details]
[PATCH] Proposed solution
Comment 4 Timothy Hatcher 2010-03-18 09:23:50 PDT
Can you explain how this fixes the bug in the ChangeLog?
Comment 5 Alexander Pavlov (apavlov) 2010-03-18 09:45:57 PDT
Created attachment 51037 [details]
[PATCH] Manual removal of empty Text nodes
Comment 6 Timothy Hatcher 2010-03-18 09:52:27 PDT
Comment on attachment 51037 [details]
[PATCH] Manual removal of empty Text nodes

> +            var sibling = this.element.firstChild;
> +            while (sibling) {
> +                var nextSibling = sibling.nextSibling;
> +                if (sibling.nodeName === "#text" && sibling.nodeValue === "")
> +                    sibling.parentNode.removeChild(sibling);
> +                sibling = nextSibling;
> +            }
> +
>              prefixTextNode.parentNode.insertBefore(this.autoCompleteElement, prefixTextNode.nextSibling);

What prevent prefixTextNode from being one of the pruned nodes? Maybe this can be put into a helper function in utilities.

Is this the only place it needs to be called?
Comment 7 Pavel Feldman 2010-03-18 09:54:05 PDT
Comment on attachment 51037 [details]
[PATCH] Manual removal of empty Text nodes

> +                if (sibling.nodeName === "#text" && sibling.nodeValue === "")

Use nodeType === Node.TEXT_NODE instead.
Comment 8 Alexander Pavlov (apavlov) 2010-03-18 10:14:27 PDT
Created attachment 51042 [details]
[PATCH] Comments addressed, explanation added to ChangeLog
Comment 9 Alexander Pavlov (apavlov) 2010-03-18 10:16:41 PDT
(In reply to comment #6)
> (From update of attachment 51037 [details])
> > +            var sibling = this.element.firstChild;
> > +            while (sibling) {
> > +                var nextSibling = sibling.nextSibling;
> > +                if (sibling.nodeName === "#text" && sibling.nodeValue === "")
> > +                    sibling.parentNode.removeChild(sibling);
> > +                sibling = nextSibling;
> > +            }
> > +
> >              prefixTextNode.parentNode.insertBefore(this.autoCompleteElement, prefixTextNode.nextSibling);
> 
> What prevent prefixTextNode from being one of the pruned nodes? Maybe this can

(a) we never show completions for an empty prefix,
(b) an empty prefix can either be there from the outset (which is not the case) or after a deletion of succeeding characters (which is the case we already sanitize)

> be put into a helper function in utilities.

A good idea, done.

> Is this the only place it needs to be called?

Added a pruning call next to the other deleteContents(), to be on the safe side.
Comment 10 WebKit Commit Bot 2010-03-19 16:07:13 PDT
Comment on attachment 51042 [details]
[PATCH] Comments addressed, explanation added to ChangeLog

Clearing flags on attachment: 51042

Committed r56280: <http://trac.webkit.org/changeset/56280>
Comment 11 WebKit Commit Bot 2010-03-19 16:07:18 PDT
All reviewed patches have been landed.  Closing bug.