Bug 26602 - Web Inspector: caret moves past prompt in javascript console
Summary: Web Inspector: caret moves past prompt in javascript console
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-22 02:47 PDT by Pavel Feldman
Modified: 2010-03-19 16:07 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed solution (1.59 KB, patch)
2010-03-18 07:02 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Manual removal of empty Text nodes (1.48 KB, patch)
2010-03-18 09:45 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff
[PATCH] Comments addressed, explanation added to ChangeLog (2.51 KB, patch)
2010-03-18 10:14 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.